[Home]

Summary:ASTERISK-00153: [patch] Enhanced voicemail features
Reporter:bradster (bradster)Labels:
Date Opened:2003-08-23 00:43:33Date Closed:2004-09-25 02:49:41
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) advanced7_prompts.tar.gz
( 1) advanced7.diff
( 2) adv-oldstuff.tar.bz2
( 3) fixed_advanced7.diff
( 4) fwd-sounds-03-08-04.tar.bz2
( 5) vmfwdnomembers.tar.gz
( 6) voicemail_groups-03-08-04.diff
Description:This patch adds several enhanced features to Comedian Mail. These features include:

- Recording options: cancel and call operator
- End-of-recording options: accept, review, re-record
- Maintenance options: removal of short and silent messages
- Advanced options: call back, reply, envelope, outcall
- Temporary greetings
- Navigation options: jump to first/last message
- Access options: log into mailbox during greeting

I've attached a tar file with the patch, prompts, and instructions

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

Voice Prompts

- The necessary voice prompts are included, but in my voice

Caveats

- The Call Back and Outgoing Call features will not work correctly unless careful consideration is given to ensure that extension, mailbox, and Caller ID number formats match in the extensions.conf and voicemail.conf files
- There is currently no mechanism to ensure that messages are not available for playback (by the recipient) while they are still being reviewed (by the sender). In exceptional circumstances, this could result in a message being heard by the recipient and subsequently erased by the sender.
- I haven't taken advantage of the recent enhancements for fancy time/date for the purposes of the message envelope function
- At one point I had made the context for outgoing calls and the removal of very short and silent messages a configurable option, but foolishly deleted that modification, so for now these options are not configurable
Comments:By: bradster (bradster) 2003-08-23 00:47:23

BTW, I'll send along a copyright disclamer ASAP.

By: John Todd (jtodd) 2003-08-27 01:13:24

To save me some work, can you send me the text for all the prompts you want to have recorded?  I have a run of stuff to send to Allison, and I can tack these on the end.  jtodd@loligo.com

By: Brian West (bkw918) 2003-09-02 23:09:00

jtodd did you get the prompt list?  I'm ready to see these patches get integrated into *.

bkw

By: John Todd (jtodd) 2003-09-02 23:41:29

Prompts have been received, but I have not sent to Allison yet, as I batch these kinds of prompts up.  I expect it will be less than two weeks before I have them completed.

By: John Todd (jtodd) 2003-09-02 23:58:41

As a side note that I mentioned to Brad in email: I am slightly uncomfortable with the ability for voicemail2 to forward calls outside of itself.  I like voicemail being a roach motel: calls go in, but they don't come out.  This allows for better sandboxing of users in situations where voicemail may be used in unexpected ways.  I understand the desire to have a "call this user back" quick-button-press feature, but maybe that should be able to be turned on/off via a config option.

By: bradster (bradster) 2003-09-08 23:15:00

Well, I sympathize with the security issues. And I realize that the DISA app can achieve a similar thing. But if you are using Asterisk, say, simply to replace an existing voicemail system rather than an entire PBX, you will pretty much want it to be able to provide the same functionality you had before.

In commercial systems, the ability of even unauthorized outside callers to out of voicemail to the auto attendant is pretty much expected, and the ability of logged-in users to place outgoing calls is not so uncommon.

Since the outgoing calls still pass through the dialplan, it's still possible to filter them to allow/disallow calls at will, trigger the DISA app, or do whatever you want... and I think that the security concerns are manageable.

But hopefully everyone would be satisfied if the features could be turned on/off at will. In fact it would be nice if all the features could be toggled at will... you could avoid having a defunct "forward" option at home if you only have one mailbox, for example.

By: John Todd (jtodd) 2003-09-10 22:50:48

Additionally, before this goes in could you update it to handle the "fancy" new time and date stuff in say.c?  I expect this might require you to look in voicemail.conf for the "preferred" format for time/date announcements that the user has set.  Perhaps you could pre-assume that Tilghman's patches will be created if that might save you some trouble (http://bugs.digium.com/bug_view_page.php?bug_id=0000168)

By: bradster (bradster) 2003-09-12 22:31:38

I don't believe that fancy time/date is part of say.c (but rather part of app_voicemail2.c)... is that its final destination? It seems it would make more sense in say.c for more general use. Anyways, I'll have to make a set of changes to deal with the new set of prompts when available, so I'll just change the function used for date playback at the same time.

By: John Todd (jtodd) 2003-10-27 12:25:36.000-0600

I have the prompts done in Allison's voice, and will shortly make available.  Does this patch set still apply cleanly?  If not, can you update to ensure easy addition by mark?  Sorry for the delay; I don't schedule the transmissions to Allison.

By: daiy (daiy) 2003-10-28 11:41:30.000-0600

I just test it, it appears to apply ok, with some offsets.

fridge:/usr/src/a/asterisk/apps# patch app_voicemail2.c vm/aug26.diff
patching file app_voicemail2.c
Hunk #1 succeeded at 81 (offset 18 lines).
Hunk #2 succeeded at 1004 (offset 99 lines).
Hunk #3 succeeded at 1070 (offset 99 lines).
Hunk #4 succeeded at 1245 (offset 99 lines).
Hunk ASTERISK-1 succeeded at 1255 (offset 99 lines).
Hunk ASTERISK-2 succeeded at 1275 (offset 99 lines).
Hunk ASTERISK-3 succeeded at 1302 (offset 99 lines).
Hunk ASTERISK-4 succeeded at 1337 (offset 99 lines).
Hunk ASTERISK-5 succeeded at 1425 (offset 99 lines).
Hunk ASTERISK-6 succeeded at 2202 (offset 101 lines).
Hunk ASTERISK-7 succeeded at 2297 (offset -179 lines).
Hunk ASTERISK-8 succeeded at 2471 (offset -179 lines).
Hunk ASTERISK-9 succeeded at 2499 (offset -179 lines).
Hunk ASTERISK-10 succeeded at 2534 (offset -179 lines).
Hunk ASTERISK-11 succeeded at 2771 (offset -179 lines).
Hunk ASTERISK-12 succeeded at 2822 (offset -179 lines).
Hunk ASTERISK-13 succeeded at 2858 (offset -179 lines).
Hunk ASTERISK-14 succeeded at 2892 (offset -179 lines).
Hunk ASTERISK-15 succeeded at 2938 (offset -179 lines).
Hunk ASTERISK-16 succeeded at 2989 (offset -179 lines).
Hunk ASTERISK-17 succeeded at 3005 (offset -179 lines).
Hunk ASTERISK-18 succeeded at 3098 (offset -179 lines).
Hunk ASTERISK-19 succeeded at 3137 (offset -179 lines).
Hunk ASTERISK-20 succeeded at 3410 (offset -166 lines).

By: John Todd (jtodd) 2003-10-29 00:09:08.000-0600

bradster - the sound files are located in http://www.loligo.com/sounds/ - you may have to dig around to get the particular ones you wanted, but they (and others) should all be in there.  see sounds-new.txt for details.

Additionally, yes, the "fancy" date announcement stuff is in say.c, and I think these patches would benefit greatly if they supported the new updated methods for announcing dates.  However, if it's going to take a long time to implement, we'll go with what we've got and maybe Tilghman will take up the torch to fix them to work with his date methods.

By: bradster (bradster) 2003-10-30 19:34:42.000-0600

Very cool... thanks for the prompts. I'll have to wait till the weekend to take a look at this and hopefully get everything squared away then.

[Nov 4] FYI... I'm still fixing/tidying. Aiming to finish end of week hopefully.

edited on: 11-04-03 14:59

By: bradster (bradster) 2003-11-09 19:56:26.000-0600

Well, I figured I might as well break this patch up and upload it as I finish. Today I uploaded the patch and prompts for the "End-of-recording options: accept, review, re-record" bit. It should offer review and re-recording of incoming messages, as well as of greeting messages and recorded names (under mailbox options). I commented out the ability to delete and cancel an incoming message, figuring it was a bit risky at this point the way the sequencing of message numbers works, but the code is still there.

The files are:

patch.diff (7,090 bytes) 11-09-03 19:51
prompts.tar.gz (13,737 bytes) 11-09-03 19:52

I thought I had given them more descriptive names but no.

More as I complete it, and I'd certainly be interested to know anyone's experiences applying the patch.

By: Brian West (bkw918) 2003-11-10 17:31:22.000-0600

so is this ready to bet added?

By: Brian West (bkw918) 2003-11-10 17:35:49.000-0600

Doesn't cleanly apply to 11/10/2003 cvs.

By: tekati (tekati) 2003-11-10 17:45:45.000-0600

Applied the patch.

[root@asterisk apps]# patch -p0 < patch.diff
patching file app_voicemail2.c
Hunk #1 succeeded at 1040 (offset 3 lines).
Hunk #3 succeeded at 1201 (offset 3 lines).
Hunk #4 succeeded at 2265 (offset 7 lines).
Hunk ASTERISK-1 succeeded at 2957 (offset 5 lines).

I also had installed a patch to get rid of the double goodbye during a wrong password attempt which could be why the offset is happening.

Installed the sounds in the sounds directory and had to add the needed lines to the end of the sounds.txt file in order for the make install to work.

%vm-accept.gsm%accept

%vm-listen.gsm%listen

%vm-Press.gsm%Press

%vm-reachoperator.gsm%reachoperator

%vm-rerecord.gsm%rerecord

%vm-tooshort.gsm%tooshort

Tested it and all works as advertised.

By: Brian West (bkw918) 2003-11-10 17:52:01.000-0600

patching file app_voicemail2.c
Hunk #1 FAILED at 1037.
Hunk #2 FAILED at 1137.
Hunk #3 FAILED at 1198.
Hunk #4 FAILED at 2258.
Hunk ASTERISK-1 FAILED at 2952.
5 out of 5 hunks FAILED -- saving rejects to file app_voicemail2.c.rej

By: bradster (bradster) 2003-11-12 21:42:15.000-0600

Hmm... I don't know what to suggest exactly. It works fine for me for Nov 12 CVS version of app_voicemail.c

Are you sure you're using the most recent version of the patch?

By: Brian West (bkw918) 2003-11-12 22:44:58.000-0600

either way a new diff will need to be created.... app_voicemail.c went bye bye.. and app_voicemail2.c move to app_voicemail.c.

bkw

By: bradster (bradster) 2003-11-16 01:26:00.000-0600

As far as I can see,  patch.diff (7,090 bytes) 11-09-03 19:51 (which is for post-recording options) still can be applied to the new app_voicemail.c

I've also posted envelope.tar.gz (13,242 bytes) 11-16-03 01:11 which contains the patch and prompts for the message envelope feature; to use it, you have to hit '3' for advanced options (which may not be obvious) and follow the instructions. From my experience, it can be applied successfully after the Nov. 9 patch (or just by itself).

It is designed to announce the calling party's number, or recorded name if the CID number of the calling party matches the number of a mailbox in the same context and a name is recorded for that mailbox. It also announces the time/date using Tilghman's format.

By: Brian West (bkw918) 2003-11-16 15:52:39.000-0600

app_voicemail.c: In function `leave_voicemail':
app_voicemail.c:1207: warning: implicit declaration of function `play_record_review'
app_voicemail.c: At top level:
app_voicemail.c:2981: warning: `play_record_review' was used with no prototype before its definition
app_voicemail.c:2981: warning: `play_record_review' was declared implicitly `extern' and later `static'
app_voicemail.c:2278: warning: previous declaration of `play_record_review'
app_voicemail.c: In function `play_record_review':
app_voicemail.c:2992: warning: unused variable `options'
app_voicemail.c:2990: warning: `duration' might be used uninitialized in this function


those need to be fixed.

By: Brian West (bkw918) 2003-11-16 16:09:59.000-0600

Ok I tested this patch.... need to double check and ensure that you have \n's on all your ast_verbose messages because it causes some lovely wraping of the messages.  Other than that it looks good...  Maybe some more indepth documentation.

By: kbock (kbock) 2003-11-17 15:02:03.000-0600

Was the callback feature removed?  When reviewing messages (by the recipient), I'd like the option to call the person back.  If this was in the origional release, could you post it again please?  It was deleted.

By: bradster (bradster) 2003-11-19 22:25:42.000-0600

callback was not so much removed, it's just that i'm in the process of finalizing this patch bit by bit, and i asked jtodd  to remove the original outdated patches to avoid confusion. Hopefully I won't be that much longer, I'm just trying to get the context that callback calls are placed in to be sufficiently configurable.

By: bradster (bradster) 2003-12-23 11:02:40.000-0600

Okay, I think I've more or less finished my work with the following features contained in the patch in the "advanced.tar.gz" file (which contains the relevant prompts also).

- Callback
- Reply
- Envelope
- Outgoing calls

For the Callback and Outgoing calls to work, you need to specify, in voicemail.conf, a dialplan context for each mailbox, using callback= and dialout=, as options for that mailbox. If you leave these blank, the intended behaviour is that the options not be prompted or selectable.

Also for callback to work you need to of course have extensions.conf set up so that whatever format the CID number of the incoming call is in can be used to place an outgoing call.

I realize that the patch produces a warning or two when compiled, but I think everything works as intended.

Oh, and to get any of these options, you need to hit '3' - Advanced Options.

edited on: 12-23-03 10:54

By: cyb (cyb) 2004-01-10 23:35:27.000-0600

Below are some notes I made while testing this patch:

1) Applied the following to app_voicemail.c from CVS-01/09/04-23:36:28
patch.diff (7,090 bytes) 11-09-03 19:51
prompts.tar.gz (13,737 bytes) 11-09-03 19:52
=> No problems, all chunks OK
2) I then applied the patch contained in the file below on top of the patched app_voicemail.c from step 1
advanced.tar.gz (57,932 bytes) 12-23-03 10:51
=> error with last chunk but doesn't look like a problem (see the .rej below)
"
*** 3396,3398 ****
  {
        return ASTERISK_GPL_KEY;
  }
--- 3780,3788 ----
  {
        return ASTERISK_GPL_KEY;
  }
"

3) Received the following warnings on compile. Only warnings though.

app_voicemail.c: In function `leave_voicemail':
app_voicemail.c:1454: warning: implicit declaration of function `play_record_review'
app_voicemail.c: In function `vm_execmain':
app_voicemail.c:2871: warning: implicit declaration of function `dialout'
app_voicemail.c: At top level:
app_voicemail.c:3510: warning: `dialout' was used with no prototype before its definition
app_voicemail.c:3510: warning: `dialout' was declared implicitly `extern' and later `static'
app_voicemail.c:2871: warning: previous declaration of `dialout'
app_voicemail.c: In function `dialout':
app_voicemail.c:3515: warning: passing arg 1 of `strlen' from incompatible pointer type
app_voicemail.c: At top level:
app_voicemail.c:3793: warning: `play_record_review' was used with no prototype before its definition
app_voicemail.c:3793: warning: `play_record_review' was declared implicitly `extern' and later `static'
app_voicemail.c:2590: warning: previous declaration of `play_record_review'
app_voicemail.c: In function `play_record_review':
app_voicemail.c:3804: warning: unused variable `options'
app_voicemail.c:3802: warning: `duration' might be used uninitialized in this function

4) Tested the options a caller gets by pushing 'pound' after leaving a voicemail... Works as expected (1=accept, 2=listen, 3=re-record, 0=operator_extn). Excellent!

5) Modified voicemail.conf to add callback=CALLBACKCONTEXT and dialout=DIALOUTCONTEXT to test mailboxes and logged on to them. I was able to use the "advanced options" (3-1=reply, 3-2=callback, 3-3=envelope, 3-4=dialout) while listening to a message and use the dialout option when at "homestate". Very nice! It even does "fancy" dates...

Further experimentation with callback= and dialout= revealed that:
- if those options are NOT present, 3-2 callback and 3-4 dialout are still being offered and selectable but they fail (* tries to dial exten 'xxxx' in context '').
- looks like the callback= option in voicemail.conf doesn't do much... both 3-2 callback and 3-4 dialout use the context specified as dialout= to place the call.

Very minor fixes seem to be required but I will definitely be using these patches as such right away since they add very nice features (long awaited too!) to voicemail. Looking forward to seeing this work included in the CVS.

By: muckl (muckl) 2004-01-11 01:32:35.000-0600

For me, everything that cyb said is correct, too (last hunk fails, compile errors, but seems to work)

but:

While listening to a recording in the Mailbox, advanced Options (3) plus:
1 - Reply: works
2 - call the person who sent this message: doesnt work: "Im sorry, i did not understand your response"
3 - hear the message envelope: works
4 - place an outgoing call: doesnt work: "Im sorry, i did not understand your response"


I have tried to add my additional parameters in voicemail.conf to [general] and to the [client] section, both possibilities dont work for me:
; advanced voicemail options:
callback=default
dialout=default


In addition to this, after recording a message, "0" (should be operator) saves the message (same as "1")

By: bradster (bradster) 2004-01-14 11:41:35.000-0600

Thanks for the recent feedback! I've posted a couple of files:

* advanced2.tar.gz (for envelope, callback, etc) (57,947 bytes) 01-14-04 00:01
* patch2.diff (for review/rerecord after recording) (7,344 bytes) 01-14-04 01:02 - note: this needs the prompts in prompts.tar.gz

Anyways, I think I've cleaned up most of the compiler warnings, fixed the problem with the callback context not being used, and fixed the prompts not to play when there are no contexts specified.

cyb - when you were leaving the callback and dialout contexts blank, were you omitting this entirely or actually using callback=|dialout= ?  When they are left out entirely, the intention is that the user would get "I did not understand your response" or something along those lines if they select 3-2 or 3-4.

muckl - the callback and dialout contexts need to specified on a per-mailbox basis, such as

mailbox => password,name,email,pager,callback=<context>|dialout=<context>

One thing I forgot to check was what happens when you select "0" after recording a message and pressing #, but the intended behaviour was to save the message, then transfer the call to the operator.

By: cyb (cyb) 2004-01-26 00:11:38.000-0600

Applied the new patches to CVS-01/25/04-22:34:39 - compiled cleanly.

callback= and dialout= Contexts now working as expected. The callback and dialout prompts are only offered when the contexts are defined. Also, not setting the contexts correctly caused the "I did not understand your response" message to play when hitting 3-2 or 3-4. I think that I was having the context '' problem when I had callback= and dialout= as 'global' variables instead of having them as a parameter of the mailbox.

One thing I noticed though... If someone leaves a message that's too short and fails to push 1 to re-record (i.e. hangs up or pushes 0 for the operator), a msgXXXX.txt file will be left behind without any associated sound file (.gsm or .wav, ...). The next message left in the mailbox does seem to re-use that message number so nothing terribly bad happens but I haven't tested extensively (e.g. what happens if two messages are recorded simultaneously and the first one 'aborts' as described above?). I imagine that the option to cancel a message after leaving it has been commented out of the code for the same reason... Voicemail would probably need a mailbox clean/defrag proc for these options to work correctly in all cases.

-cyb

By: philipp2 (philipp2) 2004-01-26 03:49:09.000-0600

Funny that you mention that msgXXXX.txt being left behind: I just had that problem yesterday, and it was not at all related to this patch, just recent plain CVS. It curred while I was in a conversation on my BT 101 when a 2nd caller got to my voicemail but somehow didn't hang up. The CDR record shows a 7 minute connection, but there was only that msgXXXX.txt left with all but one line in it.

The nasty thing: This would cause a MWI sent to my BT 101 which then would - correctly - flash the display. Only that there is no way to get rid of this because there is no message in INBOX that could be deleted. Manual admin action to delete msgXXXX.txt is solves it, though.

By: John Todd (jtodd) 2004-01-26 17:10:18.000-0600

So, it has been mentioned to me that the way phrases are currently "edited together" with smaller words is non-optimal due to choppy nature of the sentences.

I have the following phrases which will be re-recorded by Allison. If anyone needs to update this list for this particular bug, please note them here.  I will have them recorded in the next few days.

Please enter the number you wish to call,
then press pound
year, month, day
an unknown caller
your message is too short
the number I have is
to call this number
to enter a different number
to enter a number
to compose a message
to erase your temporary greeting
to record your temporary greeting
to re-record your message
your temporary greeting
has expired
is in use
will expire
will not expire
to change the expiration date
to send a reply
to call the person who sent this message
to place an outgoing call
if this greeting should be played indefinitely
if this greeting should expire at
midnight tonight
midnight tomorrow night
another time
to hear the message envelope
say your temporary message, and then press the pound key
to accept this recording
to listen to it
to re-record it
to reach an operator
to cancel this message
Nothing has been recorded. Please speak louder to ensure a clear recording.
I'm afraid I don't know who sent this message.
You can not reply to this message because the sender does not have a mailbox.
invalid date
message from
target attendant
outside transfer
default attendant
Please enter the number to use for call transfer
Please enter the number of the extension to use for your target attendant
to use the default attendant, just press #

By: bradster (bradster) 2004-01-31 13:32:56.000-0600

Recordings of "press 1, press 2 ... press 0" would make things sound smoother than the way the press and the digit are hacked together separately now.

Also, "to record greetings" would be a helpful phrase at some point.

Have you rerecorded the existing recordings yet? If not I would make a couple of minimal changes: replace "to change the expiration date" with "to set the expiration date", replace "if this greeting should be played indefinitely" with "if this greeting should not expire", and replace "You can not reply to this message because the sender does not have a mailbox" with "There is no mailbox at this number."

By: Brian West (bkw918) 2004-01-31 14:15:11.000-0600

Brad can you look at bug 603 an tell me if your patch includes this feature.  If not talk to the person on 603 and see if we can combine them into one.  Thise patch is closer to making it into cvs once the prompts are redone.

:)

By: bradster (bradster) 2004-01-31 15:58:04.000-0600

Playback of CID is included in 156 as part of the message envelope (which is a menu option). In 603, automatic CID playback is a configurable opption.

I've only had a quick look at the code from 603, but in a nutshell, here's what I'd suggest retaining from each patch:

a) bug 603: configurable automatic playback of CID before message (actually, I'd make it individually configurable for internal/external calls... it might be nice to hear the name of internal callers spoken briefly before each message, whereas listening to the numbers rattled out might not)
b) bug 156: dtmf selectable playback of "message envelope", including CID and date/time
c) bug 156: check for a recorded name in a mailbox matching the CID number and say the name rather than the number if there is one (i.e., say the names rather than extension numbers of internal people)
d) bug 603: distinguish between internal and external contexts in voicemail config file (to have "extension" said before internal calls... which would be a limited occurrence given (c) above. However, I could see where this distinction could be useful to restrict callback calls to internal extensions, and to avoid offering the option to "reply" to messages from external callers. To this end, I would suggest that the internal rather than external contexts be listed in voicemail.conf, since the consequences of forgetting one and having an internal extension mistaken for an external number would not as bad as mistaking an external number for an internal one... toll fraud, etc. I'd be inclined, though, to make those internal/external changes in a follow up patch after 156 is part of CVS, just to avoid drawing 156 out further).

Anyways, I'll talk to the person from 603 to see what we could do about combining the CID stuff.

By: hwstar (hwstar) 2004-01-31 19:02:14.000-0600

Bradster-

I am the author of the patch for bug 603. I like what you have proposed. I'd be happy to assist you in  integrating and tesing my features per your plan. I do have some questions though:

Do you have any ideas on what would be a good syntax to use for internal/external for item a?

Do you have any ideas on what would be a good syntax to use to describe the multiple internal contexts for item d?

Steve.

By: Brian West (bkw918) 2004-01-31 20:24:56.000-0600

hwstar is it ok if I close 603 out and focus here ?

By: hwstar (hwstar) 2004-01-31 21:16:37.000-0600

bkw918-

Yes, go ahead and close out 603.

Steve.

By: hwstar (hwstar) 2004-01-31 22:45:50.000-0600

In readying my system to investigate merging my changes into the advanced features, I ran into the following after applying patch2.diff:


patching file app_voicemail.c
Hunk #1 succeeded at 85 (offset 2 lines).
Hunk #2 succeeded at 1302 (offset 19 lines).
Hunk #3 succeeded at 1385 (offset 2 lines).
Hunk #4 succeeded at 1463 (offset 19 lines).
Hunk ASTERISK-1 succeeded at 2588 (offset 4 lines).
Hunk ASTERISK-2 FAILED at 3417.
1 out of 6 hunks FAILED -- saving rejects to file app_voicemail.c.rej

Note: I applied the patch file in advanced2.tar.gz first then applied the
patch file patch2.diff. Is this the correct order to apply these patches?
The asterisk version I used was pulled from CVS today (1/31/04) at 18:17.



Here is the contents of app_voicemail.c.rej:

***************
*** 3411,3413 ****
 {
       return ASTERISK_GPL_KEY;
 }
--- 3417,3600 ----
 {
       return ASTERISK_GPL_KEY;
 }
+
+
+ static int play_record_review(struct ast_channel *chan, char *playfile, char *recordfile, int maxtime, char *fmt, int outsidecaller)
+ {
+       /* Record message & let caller review or re-record it, or set options if applicable */
+       int res = 0;
+       int cmd = 0;
+       int max_attempts = 3;
+       int attempts = 0;
+       int recorded = 0;
+       time_t start;
+       time_t end;
+       int duration = 0;

edited on: 01-31-04 22:50

edited on: 01-31-04 22:50

By: bradster (bradster) 2004-02-10 16:27:53.000-0600

You might want to try applying them in the other order. They really have nothing to do with each other. Actually, patch2.diff doesn't have anything to do with the CID playback, so you don't really need to apply it at all for this purpose.

Sorry to just get back to you know; it hasn't been the best week.

By: hwstar (hwstar) 2004-02-11 23:53:20.000-0600

Bradster - I'll try your suggestions and get back to you.

Steve.

By: hwstar (hwstar) 2004-02-15 17:59:41.000-0600

I have merged 603's features into this feature but it appears that chan->context
in leave_voicemail() does not contain the caller's context (it seems to contain the called destination context which is useless). I have tried this with both zap and IAX channels and it still returns macro_vmexten which is the called context. Here is what was recored in the .txt file in the VM spool directory:

;
; Message Information file
;
[message]
origmailbox=101
context=macro-vmexten
exten=s
priority=2
callerchan=Zap/25-1
callerid=Unknown
origdate=Sun Feb 15 02:22:29 PM PST 2004
origtime=1076883749
duration=4


Can someone tell me whether this is a bug, or the context has been coded to to be the destination context in the channel modules for good reason?

I can't see any reason why the called context matters more than the calling context for voicemail functionallity.

The only way to get the enhancement to work correctly is to somehow get the caller context. I really would not prefer change other modules outside of app_voicemail.c to make this work. Anyone have any cute ideas which could be used to get this information without modifying the channel modules?

I suppose I could throw out the notion of looking for channel context, and instead filter on the callerchan field, but I feel this is a kludge as well.

By: hwstar (hwstar) 2004-02-16 18:38:54.000-0600

I was able to get past the context problem, so here is the merged patch of advanced 2 and my stuff to say the number of the calling party. I've called it
advanced3.tar.gz. (see files above) I have tested the caller ID stuff extensively, but would appreciate some help from Bradster and the group to test the advanced features thoroughly. To install the patch, unpack the tar file, and copy the prompts into /var/lib/asterisk/sounds. Take a look at the voicemail.conf.config-saycid file for an example of how to config the saycid function. Finally, go to the apps directory, make a backup copy of app_voicemail.c, and then apply the patch as patch -p0 < advancedcidmerge.patch, recompile asterisk, and test.

By: ltropiano (ltropiano) 2004-02-16 22:53:21.000-0600

I'm a bit confused, I applied the advanced3 patches to app_voicemail.c and got the "CID" stuff... although for some reason I have the following line and don't get CID said during playback, but if I hit option 3 and envelope it works just fine...

24 => 1234,MyName,myemail@email.com,mypager@pager.com,tz=central|saycid=yes|dialout=voicemail-callback|callback=voicemail-callback

But I don't get any of the other stuff this bug report indicates... which patches should I install in what order?

By: hwstar (hwstar) 2004-02-16 23:55:31.000-0600

itropiano -

Everything you need is in the advanced3 patch EXCEPT patch2.diff mentioned above (for review/rerecord after recording). I did not integrate the patch2.diff because I could not get it to patch correctly against the latest version of Asterisk fro CVS after installing the advance2. I'll leave this for bradster to sort out.

I tested your options with a slight modification and it works correctly:

101 =>1234,MyName,myemail@email.com,,tz=pacific|saycid=yes|dialout=voicemail-callback|callback=voicemail-callback

Do you have vm_from.gsm and vm_an_unknown_number.gsm installed in /var/lib/asterisk/sounds? These files are in the tarball, and if they're missing
you will get nothing. Also, it would be good to define your internal contexts
with cidinternalcontexts in the general section.

edited on: 02-16-04 22:44

By: ltropiano (ltropiano) 2004-02-17 08:40:58.000-0600

the problem with patch2.diff is that the "duration" (start/end) timers for the voicemail message is not reset/set properly while you're in the menu, so a short message of 5 seconds was marked at 45 seconds because I spent 40 seconds in the menu rerecording..., etc.

Also the message I left was truncated a bit, not sure if this was a function on the patch.

By: ltropiano (ltropiano) 2004-02-17 08:43:24.000-0600

-rw-r--r--    1 root     root         2607 Feb 16 17:33 vm-an-unk-num.gsm
-rw-r--r--    1 root     root         1386 Feb 16 17:33 vm-from.gsm

Those two files exist in /var/lib/asterisk/sounds.  Still not getting CID played for the extension I define "saycid=yes" in.  The only difference I see in the line above that you say that works is that I have a pager-email and you don't...  BTW, I do have cidinternalcontexts defined in the general section.

By: ltropiano (ltropiano) 2004-02-17 08:48:46.000-0600

nevermind, the saycid is working... apparently new messages coming in work fine, but old saved messages might not have the proper info in the .txt file..

By: hwstar (hwstar) 2004-02-17 13:14:55.000-0600

itropiano-

Yes, I had to add the macrocontext field so that I could make the internal/external decision. If the normal contexts is a macro and the
macrocontext is missing, the CID readback will be skipped.

By: mlh (mlh) 2004-02-19 14:54:18.000-0600

Will this be added soon?

By: hwstar (hwstar) 2004-02-19 22:40:59.000-0600

mlh -

I believe Bradster should look at it and comment on it, plus it would be nice to know if you installed the patch and it worked as you expected.

Steve.

By: bradster (bradster) 2004-02-20 00:03:15.000-0600

Hi there,

Okay, I've looked at the merged patch. Looks good.

I've made some changes to the play_message_callerid so that I'll be able to use it to replace the say_calling_party function I was using for calls from the message envelope. The main thing I changed was to add, for messages that originated from an internal context, a check to see if the person's spoken name is available in lieu of their extension number, and to say the spoken name if it is available.

I've also slightly changed the way the prompts are said. It requires vm-unknown-caller and vm-from (included in prompts.tar.gz) and from-extension and from-phonenumber (which you would have to record yourself for testing).

I didn't remake a diff file for 3 reasons: (1) I still have to fully replace say_calling_party with play_message_callerid; (2) the reply function didn't work for me when I tested it so I need to look at it again; and (3) I want to look at leveraging this internal context business to prevent people from trying to reply to messages that come in from outside numbers. But I figured I'd throw this out now, especially for hwstar to take a look at the modifications I made.

** EDITED: Ugh, ok this doesn't display well, I'll attach a copy of play-message-callerid.txt -- my comments are the ones that start with BB **

static int play_message_callerid( struct ast_channel *chan, struct vm_state *vms, char *cid, char *context)
{
       int res = 0;
       int i;
       char *callerid, *name;
       char prefile[256]="";


       /* If voicemail cid is not enabled, or we didn't get cid or context from the attribute file, leave now. */
       /* BB: Still need to change this so that if this function is called by the message envelope (and someone is explicitly requesting to hear the CID), it does not check to see if CID is enabled in the config file */
       if((cid == NULL)||(context == NULL))
               return res;

       /* Strip off caller ID number from name */
       ast_log(LOG_DEBUG, "VM-CID: composite caller ID received: %s, context: %s\n", cid, context);
       ast_callerid_parse(cid, &name, &callerid);
       if((callerid != NULL)&&(!res)&&(strlen(callerid))){
               /* Check for internal contexts and only */
               /* say extension when the call didn't come from an internal context in the list */
               for(i = 0 ; i < MAX_NUM_CID_CONTEXTS ; i++){
                       ast_log(LOG_DEBUG, "VM-CID: comparing internalcontext: %s\n", cidinternalcontexts[i]);
                       if((strcmp(cidinternalcontexts[i], context) == 0))
                               break;
               }
               if(i != MAX_NUM_CID_CONTEXTS){ /* internal context? */
                       if(!res) {
                               snprintf(prefile, sizeof(prefile), "voicemail/%s/%s/greet", context, callerid);
                               if (strlen(prefile)) {
                               /* See if we can find a recorded name for this person instead of their extension number */
                                       if (ast_fileexists(prefile, NULL, NULL) > 0) {
                                               ast_verbose(VERBOSE_PREFIX_3 "Playing envelope info: CID number '%s' matches mailbox number, playing recorded name\n", callerid);
                                               res = wait_file2(chan, vms, "vm-from");
                                               res = ast_streamfile(chan, prefile, chan->language) > -1;
                                               res = ast_waitstream(chan, "");
                                       } else {
                                               ast_verbose(VERBOSE_PREFIX_3 "Playing envelope info: message from '%s'\n", callerid);
                                               /* BB: Say "from extension" as one saying to sound smoother */
                                               res = wait_file2(chan, vms, "vm-from-extension");
                                               res = ast_say_digit_str(chan, callerid, "", chan->language);
                                       }
                               }
                       }
               }

               else if (!res){
                       ast_log(LOG_DEBUG, "VM-CID: Numeric caller id: (%s)\n",callerid);
                       /* BB: Since this is all nicely figured out, why not say "from phone number" in this case" */
                       res = wait_file2(chan, vms, "vm-from-phonenumber");
                       res = ast_say_digit_str(chan, callerid, AST_DIGIT_ANY, chan->language);
               }
       }
       else{
               /* Number unknown */
               ast_log(LOG_DEBUG, "VM-CID: From an unknown number");
               if(!res)
                       /* BB: Say "from an unknown caller" as one phrase - it is already recorded by "the voice" anyhow */
                       res = wait_file2(chan, vms, "vm-unknown-caller");
       }
       return res;
}

edited on: 02-19-04 22:49

edited on: 02-19-04 22:54

By: hwstar (hwstar) 2004-02-20 10:31:23.000-0600

Bradster-

Great stuff! The only thing I would mention is if the caller's name was said instead of the extension number, the user would either have to use the reply function or look the caller's extension up manually. Couldn't you say something
like "from John Doe at extension XXX"? or do you think this might be too wordy?

Steve.

By: bradster (bradster) 2004-02-21 17:01:35.000-0600

That's a good point. I can see situations where it would be useful, and situations where it would be redundant (like in an office with 6 people).

The ideal thing, I guess, would be to configure CID playback to do one of the following things:
- name and number
- name or number (name if available, number otherwise)
- name or nothing
- number only
- nothing

And to be able to configure it for internal/external calls separately. Personally, I'd like to hear the name of internal callers said before the message, but not the phone number of external callers because it gets too lengthy.

That's just a lot of configuring to do.

Anyways, in the meantime, I think I've got everything as it is working correctly, and the two patches integrated into one (so the review after recording a message functionality is now included, in advanced4.diff)

Brad

By: hwstar (hwstar) 2004-02-22 00:53:59.000-0600

Brad,

I tested your patch and the CID reporting works as advertised. The review feature works when called from an internal extension, or after a dial timeout to voice mail from an incoming trunk, but there appears to be an interaction with the DISA app when the caller presses 1 to accept the message they recorded.

After getting the "message saved" prompt, I get a "your call cannot be completed as dialed" which in my configuration means that something attempted to place an outgoing call but the number pattern dialed did not match anything in my outgoing context.

I use DISA to avoid ringing all of the phones in my house during testing since I can direct a call to a specific extension. As a sidenote, the prior version without the review feature worked fine in conjunction with DISA.

I really don't know if this is serious enough issue to go after since most people aren't going to use it this way, but it is worth mentioning.

Steve.

edited on: 02-21-04 23:43

By: hcb (hcb) 2004-02-22 07:46:50.000-0600

Brad,

thanks so much for this patch! This is great! I tried it with the current CVS and the advanced4.diff. Everything worked, except the log into mailbox during greeting. I can press # to skip the message and starting recording. But * or any number does not give a password prompt or anything.

Christian.

By: ltropiano (ltropiano) 2004-02-22 09:02:58.000-0600

Besides from changing the vm-*.gsm filenames it calls from previous patch, the other problem I've found is that if I press # after recording a message and spend some time in the review menu, the "time" for the actual message (ie. the number of minutes, seconds) gets skewed with the time from the message and the time spent in the menu.  So a 8 second message with 30 seconds in the review section, would be recorded as a length of 0:38 in the attached e-mail or page.

It would be nice to have a feature to say what number is considered "unknown" -- I have some logic in my extensions that if the message is from an unknown caller, it changes the CID to "Unknown <000-000-0000>" ... instead of hearing message from zero, zero, zero, zero ...

By: ltropiano (ltropiano) 2004-02-22 09:16:51.000-0600

It also would be nice to turn off the "reach operator" functionality, not every company has operators on duty.

By: hwstar (hwstar) 2004-02-22 11:48:38.000-0600

Brad,

What about the notion of being able to enable or disable the review feature per extension?

I can see this being useful in situations where you might want no review
functionallity for residential users, because the callers tend to dislike being prompted more than necessary.

For commercial users, Customer service in a company might want to turn this off because the messages can get quite lengthy, but other users might want it on to allow the caller to correct mistakes if most of thier messages are short.

Steve.

By: bradster (bradster) 2004-02-24 15:39:30.000-0600

Hi, thanks for the feedback.

- Enable/disable review: I'll work on programming this
- Enable/disable reach operator: I'll work on programming that, also
- Message duration incorrectly including time during review: I'll try to figure out the problem, though if anyone has any idea, I'd be happy to hear it
- Logic to make 000-000-0000 read out as unknown - my (respectful) suggestion would be to modify extensions.conf so that a CID of 0000000000 is changed to unknown before going to voicemail
- interaction with DISA: I'm clueless on this one

As for '*' to log in to your mailbox, once you reach a mailbox, I can see several things you might want to do:
- cancel leaving that message and enter a different mailbox to leave a message for
- cancel leaving that message and return to where you were before voicemail (e.g., the automated attendant)
- log into your own mailbox

And it's hard to come up with a series of keystrokes that pleases everyone.

Maybe what I should do is make '*' do basically the same thing as '0', transfer to a specific extension, like 'a', meaning that someone has escaped voicemail and wants something other than the operator, and people can do what they want using extensions.conf (login, transfer, etc). Don't know. Thoughts?

By: jjanzer (jjanzer) 2004-03-01 13:49:55.000-0600

I wasn't able to locate "vm-from-phonenumber", should the line in question be "vm-from", is this file just missing from the tar.gz's, or am I an idiot and didn't see it?

By: bradster (bradster) 2004-03-07 19:34:21.000-0600

vm-from-phonenumber and vm-from-extension were added to the code to be more specific after the prompts were recorded, so there is no prompt file for that... copying vm-from to vm-from-phonenumber and vm-from-extension would be my suggested workaround for that.

I've posted advanced5.diff, which I think fixes/does the following:

- requires review=yes and operator=yes (in mailbox options, the same way as saycid=yes) to enable review of recorded messages and prompting for the operator, respectively [note that the operator option only changes the prompting for the operator, it doesn't change the behaviour of what happens when you actually press "0", maintaining the behaviour of the current CVS version where voicemail tries to transfer you even if there is no "o" extension defined in extensions.conf]

- message durations should be correct regardless of whether the message is reviewed or rerecorded
- "*" if you press it before the beep to begin recording will escape out of voicemail to extension 'a', the same way "0" would go to extension 'o'

edited on: 03-08-04 14:55

By: jjanzer (jjanzer) 2004-03-08 11:38:18.000-0600

Note: looks like my filenames were mangled sorry, I can re-upload them with a different name if people care...

Anyway,
Here is my patch to turn on voicemail groups: voicemail_groups-03-08-04.diff  (called 03-08-04.diff after the upload)
Here are the GSMs: fwd-sounds-03-08-04.tar.bz2 (sounds-03-08-04.tar.bz2)

What it does is add support to allow a user to create a new message or forward a message to multiple users. It lets them attach in 3 methods, add-by-group, add-by-lastname, add-by-extension.

Groups are very easy to add to the voicemail.conf all you have to do is define the [groups] context like so:


[groups]
Engineering => 1|4001@corporate,4002@corporate|groups/grp-engineering
CustomerService => 2|4003@corporate|groups/grp-customerservice


First arg is the id that the user must enter to select a group
Second arg is a list of users in the group seperated by ","
Third arg is where the audio file is for their greeting, if they don't have one, then their group id (the first arg) is played by reading the digits.


I just noticed bradster's new patch (5) and I will be attempting to merge my stuff with his later on today or tomorrow, this patch assumes that you have revision 1.60 of app_voicemail.c from cvs.

Note: I have mangled some of the advanced/patch features (like dialout) I will have to find some more elegant solutions in the next few days. I just figured that I should get this out sooner than later so people can take a look at it.

Let me know if there are any problems.

By: bradster (bradster) 2004-03-08 16:02:28.000-0600

hwstar -

Hey Steve,
Did you make changes to play_message_datetime as part of your patch? We seem to have somehow gotten tangled up with changes to that part of the code with the patch as it is now. Can you take a look and see what you think?
thanks, B.

By: hwstar (hwstar) 2004-03-08 19:32:24.000-0600

Brad,

Yes, My patch did change date time to open the message attribute file in the calling function so that it did not have to be opened twice.(Once for CID, Once for date/time). When I merged your patch with mine, I changed your function call to play_message_datetime to be compatible with the new scheme. I take it now there is a third feature being merged which calls play_message_datetime using the original method and it is causing a problem.

I suppose we could change it back at the expense of some inefficiency.
What do you think?

edited on: 03-08-04 22:16

By: bradster (bradster) 2004-03-10 16:42:17.000-0600

Hi Steve,

No I think everything is actually ok... I just wanted to make sure that the changes in play_message_datetime were intentional (since I wasn't really clear what was going on).

Cheers,
Brad

By: Olle Johansson (oej) 2004-04-06 03:20:05

There's so many files in here... Can I remove some of them to make it easier for testers?

Is this patch stable? Tested by many users? Documented?

By: jjanzer (jjanzer) 2004-04-06 11:33:16

I have been using 03-08-04.diff (which is advanced4 merged with my group voicemail stuff) for about a month. Everything seems pretty stable there (we have about 30 phones/users). I haven't tried advanced5 yet.

I did notice that I need to change the way groups are defined since you will eventually run out of buffer space for a single line...

By: twisted (twisted) 2004-04-06 19:48:25

A couple problems I've noticed with advanced5.diff.  When it goes to read the CID info from msgxxxxx.txt, if there are any special chararcters in the CID Number, such as ()'s or -'s it attempts to SayDigit them, instead of discarding them.  

This should be handled correctly, IMHO.

By: twisted (twisted) 2004-04-06 19:56:32

Also, when Playing back, it doesn't include the option for "Press 3 for Advanced options"  This would be a good feature to have in the VMMain menu.   In regards to the rest of the functionality, callback doesn't appear to work at all (it drops back to the message options menu when you hit 1 to confirm the number), otherwise, it patches cleanly against cvs from today 04/06/04.

edited on: 04-06-04 19:00

By: hwstar (hwstar) 2004-04-07 00:34:57

twisted-

The root of this problem is not in app_voicemail.c. If you take a look at the ast_say_digit_str() function in say.c ANY string passed to this function which contains anything other than {0-9,*,#} will cause ast_say_digit_str() to attempt to stream a file which doesn't exist.

I feel the best way to fix this is to correct the problem in ast_say_digit_str(), and skip over anything which doesn't have a matching
file name in the digits subdirectory.

Does anyone else think differently??

Steve.

By: twisted (twisted) 2004-04-07 03:44:49

I agree completely.  If it's not actually a digit, don't even TRY to play it. ast_say_digit_str() should just ignore it if it's not a digit. that's why it's called 'say digit'!

By: hwstar (hwstar) 2004-04-07 11:34:32

twisted-

Should I submit the ast_say_digit_str() issue as a separate bug fix with its own bug ID? I'd like to minimize the number of files involved with the voice mail enhancements as there are quite a few already, and this issue affects all callers of ast_say_digit_str().

Steve.

By: Steve Murphy (murf) 2004-04-07 12:44:40

I've been testing this package. Found some problems you will want to fix!

1. CRASH-- Get into the directory by last name code. To do this, follow
  roughly these steps (from memory) Voicemail Main. Advanced options.
  Send message (5?). record message. Send to individual. By Last name.
  Hit * on ALL the presented names. Crash after last possibility is
  presented, and none were selected.

2. Missing sound files!  The tar file of necc. recorded file is short a few.
  1. vm-Press   (in prompts.tar.gz )  (in advanced.tar.gz )
  2. vm-starmain  (in advanced.tar.gz ) (in envelope.tar.gz)
  3. vm-accept   (in prompts.tar.gz )
  4. vm-message-from  (in advanced.tar.gz ) (in envelope.tar.gz)
  5. vm-unknown-caller  (in advanced.tar.gz ) (in envelope.tar.gz)
  6. vm-callsender  (in advanced2.tar.gz )
  7. vm-envelope  (in advanced.tar.gz ) (in envelope.tar.gz)
  8. vm-outgoing  (in advanced.tar.gz )
  9. vm-dialout  (in advanced.tar.gz )
 10. vm-enter-num-to-call  (in advanced.tar.gz )
 11. vm-then-pound  (in advanced.tar.gz )
 12. vm-star-cancel  (in advanced.tar.gz )
 13. vm-fwd-nomembers
 14. vm-followedbythepoundkey
 15. vm-num-i-have  (in advanced.tar.gz )
 16. vm-call-this-num  (in advanced.tar.gz )
 17. vm-call-diff-num  (in advanced.tar.gz )
 18. vm-nonumber  (in advanced.tar.gz )
 19. vm-toenternumber  (in advanced.tar.gz )
 20. vm-star-cancel
 21. vm-nobox  (in advanced.tar.gz )
 22. vm-tooshort   (in prompts.tar.gz )
 23. vm-nothingrecorded
 24. vm-speakup
 25. vm-listen   (in prompts.tar.gz )
 26. vm-rerecord   (in prompts.tar.gz )
 27. vm-reachoperator   (in prompts.tar.gz )
 28. vm-cancelmessage

I've noted that most of these are in previous tar files submitted, but
not all, I think. It's kind of a hassle pulling together 4 or 5 tar files
with different/redundant versions of the same files to get what I need.
My advise: gather all the sound files together into a single file, and
attach it to this bug


3. Directory by last name doesn't work right! For instance, at my site, I
  have 9 people with last name of "Murphy". If I enter 687 after the prompt,
  I get "dir-nomatch" played every time. However, if I enter 687 before the
  prompt is finished, it will find the matches. What gives?

4. Strange Waits. Maybe due to sound files missing. Like, there is a pause,
  a fairly long one, after you record your message, just after the "thank you"
  is played.

I kinda think the  features are worth adding to asterisk, but the implementation needs some more polishing. Make it easy for your testers. Group all your
sound files so we don't have to go searching for missing items.

By: Steve Murphy (murf) 2004-04-07 12:58:31

Sorry about the above--
Was a bit rash on the first one, checked back thru the tars,
issued a second version of it.

By: Olle Johansson (oej) 2004-04-07 15:10:27

Could someone please tell me which files should stay and which files I can remove from this bug to make it easier for other users to download and evaluate?

By: hwstar (hwstar) 2004-04-07 16:21:53

oej-

I'd love to provide it to you, but I dont have all the pieces myself. We need
Bradster's input to wrap this one up.

bradster-

We need to build a final set of files in one tar archive for testing. There are a lot of testers who are waiting for something they can load and test without having to go to hither and yon to collect it all. Maybe you could help by providing your latest code and voices all tarred up in one archive.

Steve.

edited on: 04-07-04 15:14

By: twisted (twisted) 2004-04-07 17:57:27

Mmkay.. That makes sense for SayDigit, but I think the larger issue of it neglecting to dial out once you confirm the number has been overlooked completely.  Either that or I'm missing something.  I have specified contexts that DO exist to send callbacks and outdials to.  The outdial (option 4 to make a call) works, however, the option to call the number back does not, even after confirmation.

By: jjanzer (jjanzer) 2004-04-07 17:58:30

murf, The crash is caused by the lack of last name. I already caught it and fixed it on our own system, but forgot to post it back here. Decided I'd just let it go and would merge it with the advanced5.diff

to temporarily fix it just change the ast_vm_directory_main function where it states: "
//last word
pos = strrchr(tmp_user->fullname, ' '); //jump to the last space
pos++;

to:
//last word
pos = strrchr(tmp_user->fullname, ' '); //jump to the last space
if(pos) //only move the pointer if we actually got a last name
pos++;

Yeah I know it's not in a patch....

the vm-fwd-nomembers is in the vmfwdnomembers.tar.gz file I just uploaded. As far as the other sounds, murf, they are (or at least 99%) in some of the other tarballs... I forget which one though sorry.

By: Steve Murphy (murf) 2004-04-08 00:42:50

downloaded and installed the vmfwdnomem gsm file. Patched the source as indicated.
Will double check the functionality later. Found more problems:

5. I added the following to the bottom of my /etc/asterisk/voicemail.conf file:

[groups]
;format
;GROUPNAME,GROUPANNOUNCEMSG,MEMBER1@context,MEMBER2@context,MEMBER3@context,...
;example:
;;Engineering,vm-engineeringgroup,4001@corporate,4002@corporate,4006@corporate,4010@someother

Family,murphy-family,1@default,2@default,4@default,5@default,6@default,7@default,80@default,81@default
kids,murphy-kids,4@default,5@default,6@default,7@default,80@default,81@default

I also recorded and placed the "murphy-family.gsm" and "murphy-kids.gsm" files in /var/lib/asterisk/sounds,

And, after a restart of asterisk, I type "show voicemail groups" to the asterisk
CLI, and I get:

There are no voicemail groups loaded.

I removed the @default parts and tried again, no change in behavior.

I am either not doing something right, or there is a bug. Or, are groups not yet
implemented?


6. Typing in the new "show voicemail groups"  "show voicemail zones" crashes
the gastman program, when entered into its Command Window CLI prompt. Don't
know if this is because of something in the voicemail app, or just that
gastman needs some corresponding mods to work right with these...


As to (4) in the previous post, I think the delay is because it's encoding
the recorded message into a file, or mailing it, or something. Anyway, on
my machine it spends a good 5-15 seconds doing this. Perhaps you could
warn the user of the impending wait?

As to the tarballs, I wrote the first report, hit submit, then thought...
maybe they are in the other tarballs. So I hit "Stop" (too late), and
then downloaded and extracted every other tarball. I then labeled all the
files I saw the sound file listed in. (except the advanced2, as it had
mostly the same thing as advanced). OOps, just checked again, and see
I missed the advanced_3 tarball. So I just checked it. It added
2 more files to the list, vm-an-unk-num.gsm, and vm-from.gsm, with either
already existed, or aren't referenced from the current version of
app_voicemail.c. So there are still 3 files, I think, that are referenced
in app_voicemail.c, that are not in the sounds dir.

By: jjanzer (jjanzer) 2004-04-08 11:44:29

murf,
Groups are implemented like so (see my bugnote added on "03-08-04 11:38"):

[groups]
Family => 31337|1@default,2@default,4@default,5@default,6@default,7@default,80@default,81@default
kids,murphy-kids,4@default,5@default,6@default,7@default,80@default,81@default|murphy-family.gsm

Try that and let me know what happens... btw you can find me on irc @ freenode or openirc.net either as jjanzer or zeta...

Not sure about the gastman crash, I'll have to take a look at it when I have time (which could be awhile)...

By: jjanzer (jjanzer) 2004-04-08 11:52:39

Bradster can you meet me on irc or send me an email "zeta at lanthera.net". I want to make sure you don't have another patch in the wire before I merge my updates with advanced5.

By: Steve Murphy (murf) 2004-04-08 12:00:30

Another bug. or two.

7. Turned on all options (saycid, callback,dialout,tz) for my mailbox.
  the voicemail Crashed on listening to a message:

Playing vm-received (language 'en')
Playing digits/yesterday
Playing digits/at
Playing digits/10
Playing digits/50
Playing digits/4
Playing digits/p-m
Playing vm-message-from
file.c:462 ast_openstream: File digits/(does not exist in any format)
file.c:750 ast_streamfile: Unable to open digits/(  (format UNKN): No such file or directory

at this point, the voicemail app abruptly terminates, and I get Congestion.

I take it that this is an artifact of the "saycid" -- apparently it can't
handle whatever format the CID is in...? I see in the docs:

"It is designed to announce the calling party's number, or recorded name if the CID number of the calling party matches the number of a mailbox in the same context and a name is recorded for that mailbox. It also announces the time/date using Tilghman's format."

So, perhaps some documentation or example as to a "proper" format for the CID,
so that say-cid won't crash? Perhaps it's better just to make say-cid
bulletproof?

8. Undocumented config file options.
  Checking the source I see that these two options aren't documented
  anywhere in the voicemail.conf  example, where all the others are:

   emailtitle
   cidinternalcontexts

   and also, there are database related options that aren't mentioned,
   but I guess, if you don't have the proper -DUSE...VM compiled in,
   it won't matter:
     mysql:  dbuser, dbpass,  dbhost,  dbname
     postgres:   dboption

also, I note that the "silencethreshold" config file entry has a source
specified default of 256, but the config file sets it to 128... implying
that is the default...

And, finally, the voicemail.conf example file needs to document the
available options, here is what I stuck in my voicemail.conf file as
documentation... you'll have to fill it in!

; Each mailbox is listed in the form <mailbox>=<password>,<name>,<email>,<pager_email>,<options>
; if the e-mail is specified, a message will be sent when a message is
; received, to the given mailbox. If pager is specified, a message will be sent there as well.
;
; options:
;   callback=<context>   --  allow the user to do a callback from within vm. The Context should
                            be able to handle dialing the CID in whatever format in it is in.
;   dialout=<context>   --  ??
;   saycid=<yes|no>      -- should the CID digits be pronounced on the vm
;   tz=<zonemessagename>  -- which of the above time zone messages to use
;   attach=<yes|no>     -- override the global attach option for this user
;   serveremail=<address> -- override the serveremail for this user
;


-------------------------------------
More on (5) ... group config problems: I see these messages in the
/var/log/asterisk/messages file:

Apr  8 09:02:53 WARNING[1074493184]: No '=' (equal sign) in line 116 of voicemail.conf
Apr  8 09:02:53 WARNING[1074493184]: No '=' (equal sign) in line 117 of voicemail.conf

these two lines are the two group specs I put in my voicemail conf. It seems
to be thinking they are comments are something. I thought maybe the code
doesn't handle the comments, so I moved them above the [groups] line. No change.

By: Steve Murphy (murf) 2004-04-08 12:03:42

OOps, didn't update before sending the above. Will modify the group definitions.
Uh, this is a documentation bug, then. I will try it out.

By: Steve Murphy (murf) 2004-04-08 12:30:45

Ok, modified my group definitions. Didn't notice the bugnote referenced, sorry!
I was just going from I read in the source...
So, the comments in the source and the voicemail config file example should be updated.

By: Steve Murphy (murf) 2004-04-08 13:47:54

Just so I could continue testing and not crash on the callerid whatever
garbage it might contain, I made the following mods:

I added the following function to app_voicemail.c:

static void clean_callerid_num(char *cid)
{
       char *p1,*p2;

       p1 = p2 = cid;
       while( p1 && *p1 )
       {
               if( p1 && *p1 && *p1 >= '0' && *p1 <= '9' )
               {
                       /* acceptable, pronouncable digits */
                       *p2++ = *p1++;
               }
               else
               {
                       /* not a digit -- skip it */
                       p1++;
               }
       }
       *p2 = 0; /* end the string */
}

I inserted it just above the play_message_callerid function. It clears
everything but digits from the callerid. That way, you don't crash
trying to pronounce parens, brackets, dashes, spaces, etc.

Now, here is a shameless plug for a NEW FEATURE!!!

I've submitted some code to upgrade and fill in the Privacy feature, see
bug http://bugs.digium.com/bug_view_page.php?bug_id=0000752
-- now, part of that enhancement involved storing the "intros" from
callers in the dir /var/lib/asterisk/sounds/priv-callerintros/.
These files have names of the CID number, stripped clean as by the above,
in gsm format. So, it seems a shame not to use them if they exist.

I modified the code to "saycid" in the play_message_callerid function
to do this:

instead of just pronouncing the digits right off using "ast_say_digit_str",
do this instead:

       clean_callerid_num(callerid);
       snprintf(prefile,sizeof(prefile), "priv-callerintros/%s", callerid);
       if( strlen(prefile) ) {
               if (ast_fileexists(prefile, NULL, NULL) > 0) {
                       ast_verbose(VERBOSE_PREFIX_3 "Playing priv-callerintros info: CID number '%s' matches existing entry, playing recorded name\n
", callerid);
                       res = ast_streamfile(chan, prefile, chan->language) > -1;
                       res = ast_waitstream(chan, "");
               } else {
                       ast_verbose(VERBOSE_PREFIX_3 "Playing envelope info: message from '%s'\n", callerid);
                       /* BB: Say "from extension" as one saying to sound smoother */
                       res = ast_say_digit_str(chan, callerid, "", chan->language);
               }
       }

clean_callerid_num assumes you have a copy of the callerid num in a buffer
that can be modified in place. This seems to be a good assumption, you
have to parse the callerid into a name and num anyway, and that usually
into temp buffers that won't be hurt by maybe being shortened a little.

Whatcha think? Won't hurt anything to do this, if there's nothing there,
and give you a neat feature if there is.

There are 7 places in the code where ast_say_digit_str is used. They all
have to be investigated to see if raw callerid's will cause problems.
The code above to clean the callerid, coupled with the 11th hour check
to see if there's a priv-callerintros file instead could be encapsulated
into a function and called instead... ?

murf

By: Steve Murphy (murf) 2004-04-08 15:50:44

Verified that the bugfix described above prevents the crash (bug 1 that
i reported above).
With the config file group entries corrected, I verified the group
code works OK.
Perhaps I didn't get the recordings installed in the sounds dir
in the right order... but it seems like the recorded prompts aren't
giving me all the choices I really have during the voicemailmain.
Individual options presented after listening to a message, don't
announce anything for keys less than 5. I'm not going to stress over this
until I have a single tar file with all the gsm's in it necc. for this
package. I'll remove all the sounds I've added and reinstall. Maybe then
it'll work right.

By: rthrash (rthrash) 2004-04-12 13:01:52

Any progress on getting a single download for testing? I'd love to see this functionality in the standard distribution, and am ready to test and report any findings.

By: bradster (bradster) 2004-04-13 12:56:12

Wow, um, sorry to have just wandered away from this, I'm just checking my email for the first time in a week or so... I'll try to catch up on my bugnote reading in the next day or two.

bradley (at) bergman (dot) ca is my email if anyone wants to reach me directly.

By: twisted (twisted) 2004-04-18 01:34:39

HouseKeeping note - Please do not delete this - It REALLY should be considered for approval once stabalized.

By: hwstar (hwstar) 2004-04-18 21:03:12

As a sidenote for everyone involved in debugging this feature, I submitted a patch as bug 1443 to prevent ast_say_digit_str() from aborting mid string when there are non-digit characters in the caller ID string. As I mentioned in a prior post, I felt it was was best fixed in the function which did the actual work, and not in the
app_voicemail.c module.

edited on: 04-18-04 19:58

By: twisted (twisted) 2004-04-20 07:07:33

I have uploaded unoff_advnaced6.diff which patches against current cvs-head as of 04/19/04, includes the above fix, and now offers menu option when listening to voicemail when included sound file (vm-advopts.gsm) is put into /var/lib/asterisk/sounds.  

Let me know what you think... Hunk #1 failed on voicemail from advanced5.diff on today's cvs, so i fixed all that, fixed callbacks, and added the advanced options to the playback menu.  

Enjoy!

Note: I deleted the previous bugnote because *I* was in error on the fix for the callback.  it worked, but locked you in where you couldn't exit.  unoff_advanced6.diff fixes that, too.

edited on: 04-20-04 06:14

By: Steve Murphy (murf) 2004-04-20 12:57:57

Fetched, applied, and studied the diffs of unoff_advance6.diff, and the
previous  03-08-04.diff, and notice that the "group" stuff has been
removed? Any special reason? I took some trouble to test that stuff.

murf

By: bradster (bradster) 2004-04-20 14:16:31

Okay... back at this after a couple of ugly weeks.

Very nice to have callback working properly! I made one small change to unoff_advanced6.diff, to make the prompts more logical (during callback, it was
saying "the number I have is... message from... 555-1212").

I'll work on getting a tar file of prompts together today.

There are a few prompts that would make things sound slicker, but that we can also live without:
"from extension"
"from phone number"
"press 1", "press 2", ... "press 5"

I don't really know anything about the features from 03-08-04.

By: Steve Murphy (murf) 2004-04-20 15:11:02

Heh, Heh. I've suddenly come to the realization that there's a bunch
of you working on this... Duh!!!
And there's no "File Added" entry in the history below for the 03-08-04 diff
file... but it LOOKS like jjanzer.
Who's coordinating all this? jjanzer? bradster? twisted?
Help unconfuse me as to which patch is the "Real" patch that will end up
in CVS. The group stuff in the 030804 stuff seemed a nice feature.
Will it make it into the CVS as part of the app?
Can the search thru priv-callerintros be included for announcing
the call?  Heck, if you are desparate enough, and the CallerID has
a non-empty string, you can even have Festival announce the caller
at the beginning of the message....

By: bradster (bradster) 2004-04-20 18:05:22

This file contains (I hope) all of the prompts needed for advanced7.diff:

advanced7_prompts.tar.gz (70,842 bytes) 04-20-04 18:03

Some of them are actually duplicates, because the prompts saying the ideal thing don't exist, for example, vm-from, vm-from-extension, and vm-from-phonenumber all say "message from".

By: bradster (bradster) 2004-04-20 18:19:20

Personally, I have nothing against the extra features from 03-08-04, but I'd suggest keeping things as separate as possible. There are some features I'd like to add to the advancedX line of patches too, but rather than see it grow indefinitely, it might be better to ensure all it's current functionality works, get it in CVS, and go from there. Just IMHO.

By: hwstar (hwstar) 2004-04-20 23:14:38

I get a segfault when attempting to record the unavailable, busy, or name messages. To reproduce:

1. Dial voicemail extension
2. Enter password
3. Press '0' to get to mailbox options
4. Press 1,2, or 3 - asterisk segfaults with:

   -- Re-recording the message
/etc/rc.d/asterisker: line 7:  8920 Segmentation fault      /usr/sbin/asterisk -c
Ouch ... error while writing audio data: : Broken pipe
Ouch ... error while writing audio data: : Broken pipe
Asterisk CVS-04/20/04-18:21:40, Copyright (C) 1999-2001 Linux Support Services,
Inc.
Written by Mark Spencer <markster@linux-support.net>

Other than that I have tested all options (except the callback feature) and they appear to work as advertised.

advanced7.diff was installed and compiled, and and advanced7-prompts.tar.gz was  unpacked and installed.

Steve.

By: twisted (twisted) 2004-04-21 00:38:07

bradster: for the sake of sanity, I have grouped everything (with the exception of the grouping files [03-08-04*] and the most recent diff[s]) into one bz2 called adv-oldstuff.tar.bz2.  Hopefully, this will cut down on the confusion a bit.

edited on: 04-21-04 00:08

By: twisted (twisted) 2004-04-21 00:50:36

Also, I uploaded fixed_advanced7.diff that derefrences the null pointer, which was causing the segfaults mentioned above when recording OGM's.

By: Olle Johansson (oej) 2004-04-21 02:46:11

Some notes:

* Housekeeping wonders if someone could add a list of the files that we can delete and instructions for the rest of the files. I want to mail a request for testing out to the mailing list, but as long as I can't easily figure out what to download and install myself, I will not do that.

* From reading the bug reports, this patch is still in development with changes and additional features added.

* According to the current release plan, we will not add major new functions or changes in 1.0/1.1 so don't be upset if nothing happens in those releases. We need to test this a lot and get confirmations that it works and is stable, voicemail is such a critical app in asterisk.

* There are a lot of additions that we will include in a new development test platform after 1.0/1.1, this may go in there. The new dev branch will probably be named "1.3" and include some major rewrites and additional functions.

* If you have questions, comments or want to convince me or other bug marshals that I'm totally wrong, we're usually available in the #asterisk-bugs channel on IRC

By: bradster (bradster) 2004-04-21 13:56:46

I would suggest that we not develop further or change the "advanced" line of patches, just test and fix as needed. Some of the other aspects of the patch that I removed (ages ago) to keep the scope more sane I will try to put in another patch, another day. Perhaps someone else would like to do the same with the other features of 03-08-04? With that in mind, it would be helpful if someone could update the description and add'l info to just say what is below.

Also, the only files needed are the most recent prompts and advancedX files, which are currently:
advanced7_prompts.tar.gz (70,842 bytes) 04-20-04 18:03
fixed_advanced7.diff (33,560 bytes) 04-21-04 00:49

I would favour, however, retaining the collection of "old stuff".

I guess there is currently no documentation on how to configure the new features? Or is there?


---

Description  This patch adds several enhanced features to Comedian Mail. These features include:

- Recording options: cancel and call operator
- End-of-recording options: accept, review, re-record
- Advanced options: call back, reply, envelope, outcall

I've attached a tar file with the patch, prompts, and instructions  

Additional Information  

Caveats

- The Call Back and Outgoing Call features will not work correctly unless careful consideration is given to ensure that extension, mailbox, and Caller ID number formats match in the extensions.conf and voicemail.conf files
- There is currently no mechanism to ensure that messages are not available for playback (by the recipient) while they are still being reviewed (by the sender). In exceptional circumstances, this could result in a message being heard by the recipient and subsequently erased by the sender.

By: Olle Johansson (oej) 2004-04-21 14:08:06

Let's stabilize this patch here and make it stable for testing and later commit.
It's been around for ages.

I suggest
* Open a new bug report
* Add _only_ the needed files
* Writa a good text on how to install this
* Write a good instructional text of the changes in usage and configuration
* Close this bug referring to the new
* Mail to the -users list ask for massive torture tests

This bug report is very long and hard to understand now, so let's clean it up by moving to a new territory.

Also, please refrain from adding new functions. let's stop here and move this version forward. If you are creative and want to add new stuff, do it in a new bug report, please.

By: Steve Murphy (murf) 2004-04-21 18:48:18

OK, I've downloaded the advanced7 prompts and fixed_advance7 patch, and
put all in it's proper place, and patched the exec, and even fished out the
patch for bug ASTERISK-1428, and applied it to keep the say_digits from crashing
the apps on normal CID's.

When I test on an empty VM box, I choose 3, advanced options, 4, to dial out,
put in a number, and then hit '*' to cancel, as the prompt says I can,
and it then tries to dial the number with an '*' at the end!

murf

By: hwstar (hwstar) 2004-04-21 22:46:43

1. fixed_advanced7.diff does indeed fix the segfault. Thanks twisted!

2. I was able to duplicate the issue raised by Murf. But only
  when I dialed some numeric digits before I pressed the * key.
  Pressing '*' before dialing any digits does abort the call request.
  I suppose we could search the dial string for an * and abort the call
  as well. Comments on this?

edited on: 04-21-04 21:42

By: bradster (bradster) 2004-04-22 11:33:05

Hmm... its interesting what you don't think of in advance.

I suppose we should search for the * anywhere in the string and abort the call. The problem with this is that you would have to press *, and then wait for the timeout, because you wouldn't (well, your average user wouldn't) think to press * then # to end the string.

By: hwstar (hwstar) 2004-04-22 12:58:54

Why not lose the '*' option, and say "dial your number and press # to connect your call, or press # now to abort".

Steve.

edited on: 04-22-04 11:53

By: bradster (bradster) 2004-04-23 11:29:13

Yeah I was thinking that... but then I'm not sure what makes for the most consistent experience from the end user's perspective about when to use * and #. But that's a whole other story. # to cancel is probably good for now. I'll try to get that done on the weekend and get a new bug report created as well.

By: twisted (twisted) 2004-04-25 15:47:30

Moved to 1483 to keep Mantis from blowing up, and to make it easier to manage this.