Summary: | ASTERISK-00746: [patch] preliminary upgrade to app_dial.c for Privacy | ||
Reporter: | Steve Murphy (murf) | Labels: | |
Date Opened: | 2004-01-05 12:34:24.000-0600 | Date Closed: | 2008-01-15 15:41:01.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Core/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) README.privacy ( 1) 752sounds.tar ( 2) privacy-omnibus2.diffu | |
Description: | Refer to message from Steve Murphy to asterisk-dev mailing list on 4 Jan 2004; (Digest ASTERISK-424). Basically, this is a extremely prelim attempt to flesh out the Privacy option for the Dial command. Before investing more time in coding, I need some reassurance that the proper approach is being taken. PLEASE review the attached changes (diff -u format), and give me some feedback, PLEASE. | ||
Comments: | By: zoa (zoa) 2004-01-05 15:12:37.000-0600 can you give more info on what exactly this is doing ? By: Steve Murphy (murf) 2004-01-05 16:07:06.000-0600 OK, for those who don't have access to the asterisk-dev mailing group, I've extracted the more relevant parts of what I said there, and include them here. Caveats: I just finished writing this stuff up. I haven't really even compiled the stuff yet. I'm submitting it just to see if I'm approaching the problem correctly. I could be way off, and before I start debugging and spend a week wasting my time on it, I'd at least like to know I'm in the ball park! If you think you can just plug this in and recompile asterisk, and it will work, you are totally wrong! I'm certain it's full of gaffers so big, I'll be laughed out of programming school forever. Don't waste your time. But, if you want to, and tell me about my flaws, I'll be thankful. Or, if you think you could do better, feel free to do so, whether you use my stuff for a start, or just toss it and recode it up your way, it matters not to me. Explanations: After thinking things thru, I began to fully (perhaps) comprehend why Mark added the privacy option in the dial command. It became clear to me that the point to make a decision about how to handle the call would occur after the incoming caller chose an actual extension, and before the person being called was actually bridged to the incoming caller channel. This is in the bowels of app_dial.c. So, if the privacy option is set, there are two places in the dial_exec function where you want to handle things. The first is near the beginning, where there is already code for the privacy option. In this spot, you can check the privacy db against the caller id. If it is KILL or TORTURE, there is no point in continuing the dial process. The called person is just not interested in getting this call.If the setting is DENY, then, return as if the dial returned with no answer. If UNKNOWN, then here is a good time to prompt the caller for his "introduction", and record his intro to a file, if they haven't already done so. Then, the next place to put code, is just before you bridge the two channels together. At this point, the called extension has been rung, and the called person has picked up the line. At this point, the "Announce" file is played, and if the Privacy option was set, here is a good point to insert these actions: 1. Tell the called person a caller is present, who is not in the database. Play their introduction we just recorded. 2. Give the called person a list of options: 1. Caller is OK, mark as ALLOW in db, and finish the bridge. 2. Send this caller to VM now, and forevermore. 3. Send this caller to the Torture Script context now, and forevermore. 4. Send this caller a more brief and humane telemarketer "go away and don't come back" context now, and forevermore. 5. Mark this caller as ALLOW in the db, but send them to voicemail right now. 3. Collect the user response, loop the prompt until they give a correct response. Then apply the proper action to the response. Let the call bridge for ALLOW. Act like the line was unanswered for DENY (should go to VM). Switch context to the "kill_context" for KILL, or the "torture_context" if it is TORTURE. I added two variables to the privacy.conf file, the "kill_context" and "torture_context" variables. These specify a string that should be a defined context in extensions.conf. Uh, don't think I followed the existing formatting totally, but that's pretty cosmetic. I used existing examples in other files to do some tasks, and may have included unnecc. code insodoing. And, I may be making spectacularly bad structural mistakes, and for this I beg your review, to make sure I'm just not hacking it the wrong way, and making a mess of the whole thing for myself. I don't know what the caller will hear while Asterisk converses with the target called person. Will they hear the ringing/ moh? or just Nothing? I'm hoping ringing or moh, because the called user might be a bit slow to make a decision... maybe listening to the options two or three times, etc. I haven't coded anything around this, but it may require attention, I don't know. So, is this the right approach, everyone? By: Steve Murphy (murf) 2004-01-07 13:53:05.000-0600 I have actually compiled and linked and tested the code now, and made some changes to correct some problems. I've uploaded the diff -u patch again. Some problems: first, advising the caller to use '#' to terminate his introduction is not going to work. It triggers the call transfer mechanism, and we end up getting music on hold in his recording. This could be a possible vulnerability to the integrity of the dial plan. So, I set the silence detector to 2000 milliseconds, and it terminates the recording. I've included the scripts for the play files in the code. I've adjusted them based on experiences with a few "accidental" incoming users. There is a problem with ring indication/music on hold in the time period while the "called" user is being queried for their decision on the progress of the connection. The caller gets pure silence during this period, no matter what. Any suggestions? By: Steve Murphy (murf) 2004-01-09 15:06:03.000-0600 Just in case anyone is interested, here is another installment. Guess I'm just talking to myself if no one is interested... but then who will notice? Oh, well... good notes for my own reference, I guess! Just attached the 3rd version of the app. I've conquered these problems: 1. caller gets silence while callee is being prompted for decision. I've added code to keep music on hold going. The ringing may be more difficult, have not tested that yet. I suspect that the fact that no ringing is actually going on may be an issue, and we'll have to "fake" ringing. We'll see. 2. Switching to Torture or Kill Contexts. Didn't work right. So I use the same method as busy... Torture outcomes to n+301 priority, Kill outcomes go to n+201 priority. In either case, Dial gives 0 exit status. No more config file access. 3. Called Party gets hung up after DENY, KILL, or TORTURE choice. This wasn't happening. The channel would stay open indefinitely. Now it gets hung up as expected. These problems remain: 1. The blasted '#' and 'Transfer'! How frustrating. I can't see the difference, but if you exit the Dial with 0, to simulate "no answer", you go into voicemail and cannot use '#' to end your message. If you really don't get an answer, you go the same path and CAN use the '#' to end voicemail messages. I'm missing something. And, if the CALLER hits a # key anytime during the process, he gets a short vector to "transfer", hold music, and back. 2. If the average caller is a little tongue-tied, and takes a couple seconds to start his introduction, then he/she will be cut off and a blank intro will be stored. I was thinking that maybe a little code to compare 2 sec of silence with the total length of message might override the detection and let the recording continue. Is there any quick routine to analyze a file and determine that it is silence? Or if it is mostly silence? We could simply reject such a session and let the user try again. By: Steve Murphy (murf) 2004-01-15 18:21:48.000-0600 Problem number 1 resolved: The problem is in the testing! Duh! I pick up a phone on the asterisk switch, and dial out onto the tel network, and dial the other line on the switch. As my extension dials with the T option, if I hit a # locally, asterisk is doing a transfer from the outgoing side. So, the # is not an issue. Only if you are a tired tester, not thinking clearly. Problem number 2 resolved: It's all in the prompt. The prompt was confusing the incoming caller. So, I change the prompt. All it says now, is: "At the name, please say your name... <beep>." This is remarkably better at not confusing the incoming caller. This seems affective at keeping the caller from hesitating and timing out. The two second timeout stands, and the # will also terminate the recording. The patch seems to work. I invite the user community to give me their input. I'll upload the latest source, which contains the prompts, and the .gsm files (in my own, remarkably awful and boring voice) As a sidenote, I've 'primed' the privacy database with maybe 80-100 common callers I want to come thru. I've taped intros for them. I set a new context to allow incoming callers to tape/change/listento their intros. I play the intro over the loudspeaker when the call comes in. I play the voicemail greeting when they select an extension. This is more fun than a guy should be allowed to have! ;^) By: Steve Murphy (murf) 2004-03-12 16:38:41.000-0600 OK, I've added the "call screening" option to app_dial, as well as the databased Privacy option. The databased option is still "P" or "P(X)" as before, and the Call Screening is "p" (little-p), and it basically is the Privacy feature, without a memory. In other words, it is the Privacy option in it's flow, but the database lookup on the callerid is not done, and the result is set to "UNKNOWN". So, the call goes like this: caller calls in. Just before dialing the target extension, If the caller's CID number is not in the announcement directory, they are prompted to record an announcement. It is stored in the /var/lib/asterisk/sounds/priv-callerintros/ dir as with the P() option... At this point, the callee is dialed, and if they pick up, a message is played to the target, saying that "a caller, representing themselves as...", then the recorded intro is played, and then a set of options is played. These options are stored in /var/lib/asterisk/sounds/screen-callee-options.gsm, and basically go like this: "Here are your options: Dial 1 to immediately connect to this caller. Dial 2 to send them to voicemail. Dial 3 to send them to the torture scripts. Dial 4 to give them a short, polite "Don't call us, we'll call you" sendoff. The target enters the appropriate code, and the response is as selected. No database update is performed. BUGS: When "m" is selected in the Dial options, you get the MusicOnHold, but... the ringing can still be heard as a superimposed beep by the caller... While I was away on a business trip, asterisk froze twice, because the log file grew to 2 gigabytes, with uncounted millions of messages all looking like this: Feb 11 16:57:40 WARNING[1269840688]: File file.c, Line 521 (ast_readaudio_callback): Failed to write frame Feb 11 16:57:40 WARNING[1269840688]: File file.c, Line 521 (ast_readaudio_callback): Failed to write frame Feb 11 16:57:40 WARNING[1269840688]: File file.c, Line 521 (ast_readaudio_callback): Failed to write frame Feb 11 16:57:40 WARNING[1269840688]: File file.c, Line 521 (ast_readaudio_callback): Failed to write frame Once the file reaches 2 gig, the theoretical max file size in Redhat 9, I guess, asterisk freezes. It won't unfreeze till you remove or trunc the messages logfile. I haven't been able to reproduce this in the lab. I removed the P() in the dial plan, and the problem did not recur. This sure makes it look like it's my added code. If I can locate the prob, I'll submit a fix. By: Steve Murphy (murf) 2004-04-17 13:57:05 Update-- I have been running the call screen and privacy options for a month now, and I have had no 2G freeze events. Perhaps this *was* just coincidence. I've also updated the code to follow the CVS last month, and a few times since, and perhaps this fixed the problem. As far as I can tell the option is working fine. Any complaints, anyone? murf By: Brian West (bkw918) 2004-05-03 00:46:38 Please update to latest CVS .. app_dial was recently changes. bkw By: Steve Murphy (murf) 2004-05-05 09:19:54 OK, the appdial.diffu.new.txt is against today's CVS. For the newcomer, this latest patch, plus the sound files, installed in /var/lib/asterisk/sounds, plus making an empty /var/lib/asterisk/sounds/priv-callerintros dir, are all you need to start playing with this upgrade. Uh, at some point, to be uniform someone'll either have to get Allison (or maybe Charleton Heston) (or the guys who played Lurch on the Adams Family [YOU RANG?]) to do the prompts. By: Steve Murphy (murf) 2004-05-13 09:51:39 did a cvs update today, and saw a conflict with app_dial.c Resolved it, compiled, everything appears OK. uploaded the new diffs.app_dial.newer. No changes in functionality. Just keeping up with the CVS's. By: Steve Murphy (murf) 2004-05-25 14:43:33 More news! I think I nailed down the infinite loop that fills the log file. It seems to happen if the callee hangs up while the options are being given. I've remodeled that whole chunk of code, and paid more attention to the 'abnormal' conditions. Run it thru some tests, and all seems well. The file 'privacy.diffu' has been uploaded, and all the previous versions can be ignored or deleted. The only other entry necc. to install is the gsm.tar.gz file. And, you'll most likely have to hand-create the directory /var/lib/asterisk/sounds/priv-callerintros. I've been testing the app by dialing out one line, and back in on the other. I've finally grabbed a cellphone and dialed in to test, and the annoyance of having the ringing overlay the music-on-hold does not occur, which is very nice! So, as of this rev, all the "big, bad" bugs are gone. Help me find some more, and give this stuff a test run. By: Olle Johansson (oej) 2004-05-25 15:17:38 Removed files and sent out a call for testers on -dev mailing list. /O By: Steve Murphy (murf) 2004-05-25 16:00:07 uh, sorry, just one more thing. I just uploaded a new file, "gsm2.tar.gz", which is the same as the older, "gsm.tar.gz" file, but adds the files "screen-callee-options.gsm", and "priv-scripts.txt". So, altogether there are now 4 sound files associated with this option: 1. priv-introsaved.gsm 2. priv-recordintro.gsm 3. priv-callee-options.gsm 4. screen-callee-options.gsm Now, they are all recorded in my less-than-glorious voice. The text for these prompts is in the priv-scripts.txt (as well as in the source). Big Question: how do you add this stuff to the list of things to record, next time enough moola exists to get Allison to record them? By: Olle Johansson (oej) 2004-05-25 16:07:40 Ok. I guess jtodd is the right person to ask about Allison recordings. By: Trevor Peirce (trev) 2004-06-01 05:56:52 Jun 1 02:38:33 WARNING[-1220822096]: file.c:464 ast_openstream: File priv-introsaved does not exist in any format Jun 1 02:38:33 WARNING[-1220822096]: file.c:752 ast_streamfile: Unable to open priv-introsaved (format UNKN): No such file or directory It seems that I'm missing a file. Perhaps you could add this to one of the gzip files? By: Trevor Peirce (trev) 2004-06-01 06:07:51 The caller should be prompted to record their name each time they call when caller ID information is not available. I was asked to record my name the first time, but not any longer. Also, when repeating the call handling options to the callee, the recoreded name should be played back each time. On analog lines where no call progress is available the name is played while the phone is ringing, and only the options are haerd when the callee picks up. If the recored name would be repeated with the instructions, that would eliminate this problem. Aside from those few suggestions, your improvements are looking quite nice! I've been waiting for something like this to come along. By: Steve Murphy (murf) 2004-06-01 09:58:02 My apologies about the gsm2 file not having that file. I have just uploaded a new file, gsm3.tar.gz, and it has all 5 of the necc. files. OEJ -- Could you remove the gsm.tar.gz AND the gsm2.tar.gz files, now that I have the gsm3.tar.gz in place? This will reduce confusion, I hope. I will investigate the null-CID issues and issue a new patch real soon. By: Steve Murphy (murf) 2004-06-01 11:58:57 OEJ -- messed up a little. Uploaded two "privacy_updated.diffu" files. The one I want is the latest, and is slightly bigger. Could you get rid of all the diffu's except the last, size of 23,350 bytes? Trev-- I've uploaded a new patch, size 23350, name "privacy_updated.diffu". Revert your app_dial.c to the one in CVS, and apply this new patch. I've update the callee announcement/options sequence, so it cycles over the complete sequence "I have a caller who announces themselves as: <caller intro>, you have the following options, blah... blah...". You can interrupt the sequence anywhere. If you have an interface where app_dial will just start playing the announcement to a ringing channel, and you pick it up after the caller's intro has been played, then you can hit any number greater than 5 to abort the options , get the "I'm sorry" message, and start it over. Only thing is, the sequence is only played twice, and then the caller goes to VM purgatory. ;^) But looping 3 times seems a bit long... what do y'all think? I've also attacked the NOCALLERID problem. If the callerid is blank, now it is modified to always prompt the caller for their intro. I also foresee that storing this intro in sounds/priv-callerintros/ as just "NOCALLERID" will not work in large establishments, where perhaps hundreds of callers might be dumping their intros into the same file and you'll end up hearing the wrong intro as a callee. To avoid this scenario, I've added the extension of the callee to the NOCALLERID file name. Give it a spin and see what else needs fixing! Many thanks! By: flavour (flavour) 2004-06-01 18:21:18 Sound file requests can be added to: http://voip-info.org/wiki-Asterisk+sound+files+requests By: Trevor Peirce (trev) 2004-06-01 18:27:07 Fixed one bug, but another still exists :) I am asked for my name each call, which is great. However if I talk too long and get cut off, I'm not told thanks... yet if I quickly say my name I do get a thanks. You can see below that it doesn't even try to say anything, just sends me back to the Busy condition (which is what should happen, as I only have one line available at the moment). -- Playing 'priv-recordintro' (language 'en') -- Playing 'beep' (language 'en') Jun 1 15:15:31 NOTICE[-1220961360]: app_dial.c:435 play_and_record: Recording Formats: sfmts=gsm -- x=0, open writing: priv-callerintros/NOCALLERID_35 format: gsm, 0x8c8af08 Jun 1 15:15:31 NOTICE[-1220961360]: app_dial.c:457 play_and_record: Set_silence threshold to 128, maxsilence=2000 -- Took too long, cutting it short... == Everyone is busy at this time By: Steve Murphy (murf) 2004-06-01 19:37:59 Excellent! a good bug! It's directly in the code I stole from the voicemail app. It sets outmsg to 2 when silence of sufficient duration is detected, when the user hits the '#' key, but NOT when you time out. So I inserted the assignment and now it works as it should. Which means, that the exact same bug is in the voicemail app. So I checked app_voicemail.c, and sure enough, it has the same problem. Let's see, that means that... (just looked) if you time out makeing or prepending a message, you'll not get the nifty message saying "Thank You" or whatever. I'll file a bug with patch for that app. Again, apply the new "privacy_update2.diffu" to a fresh cvs version of app_dial.c... and see if this is "the one". Oh, and flavour, thanks for the cleanup! I hate to keep bothering you guys, but you can also remove the now outdated "privacy_updated.diffu" file. Brace yourselves, there may be more. By: Trevor Peirce (trev) 2004-06-02 16:08:59 Just about there... and these are really minor issues/feature requests. a) I'm told that my name is saved away for future use. This isn't actually the case because there is no CID information. Perhaps you could chop that recording in to two pieces "Okay, thank you" and then "I have stored your name..." If CID information is present, play both, if not just play the thanks. b) It would be nice to have an option to disable the saving of names all together. In some instances, users may want to request a "fresh" recording each time a call is put through. For example, to be sure it's a certain person and not someone else calling from the same office. c) If saving of names is diabled (or no CID is available), perhaps it might be a good idea to 'uniquify' the filename further. Perhaps by adding the caller's channel name as well as the called extension? d) Perhaps also add the ability to only request a name if no CID is available. If CID is received, put the call through as normal bypassing the privacy. (Maybe you've done this and I just didn't see it?) Aside from those, I can't find any other glitches. Good work! By: Steve Murphy (murf) 2004-06-02 23:49:30 a) I'm step ahead. I've left out that little "stored" phrase from the text I handed over for the next Allison taping. It just isn't necc. I guess I could make a new gsm set that reflects this... b) disable saving intros. That's a thought. I've updated the code so that you can enter the 'n' option to the dial command options. If you do, the intro will be erased after the callee makes his selection. The caller will have to say their name every time they call. -- I added a couple lines to remove the NOCALLERID intro after it's been played to the callee, and the callee has made their choice, as it really is pretty useless after the call is over. --Whether or not the 'n' option is present. c) further qualifying the NOCALLERID file, to avoid conflicts. I guess it would be possible to have more than one caller with no callerid, trying to reach the same extension at the same approx. time... I added the channel name to the NOCALLERID file name. d) You only want to "screen" calls without CID. Interesting. I just added code to the set so that you can use 'N' option in the dial command. With that present, it will assume that PRIVACY_ALLOW was the result of a db lookup, if the caller had a CID. This option is of course orthogonal to the 'n' option. At this juncture I might add these tips: 1. Don't forget that the PrivacyManager and Zapateller options exist, with the obvious benefits of "skimming" the calls of autodialers and those who don't want to reveal their CIDs. (I even do a little testing on Privacy Manager entered numbers, to make sure they have not entered my own number as their callerid). 2. MusicOnHold, as kenalker above points out, helps mask the screening time, much better than ringing indications will. You'll have higher call retention while you make your choices, if the caller is distracted with music. So, in a second I'll upload privacy_update3.diffu and gsm4.tar.gz. Same procedure to install as previous. There is a new priv-introsaved gsm file and the text is updated in the script text file. The patch includes everything you asked for, and my testing shows that it seems to work. By: Steve Murphy (murf) 2004-06-02 23:52:36 The now outdated gsm3.tar.gz and privacy_update2.diffu can be removed (to avoid confusion). Thanks! By: Steve Murphy (murf) 2004-06-03 15:28:48 And now, to muddy the waters even more, some documentation on the whole wide-ranging subject of privacy in Asterisk. See the README.privacy. This might be dropped unceremoniously in the asterisk/doc directory... when the time comes. By: Steve Murphy (murf) 2004-06-03 15:45:31 Just submitted the privacy_update4.diffu file. Did the following therewith: 1. Changed a bunch of debugs from ast_log() calls to ast_verbose(VERBOSE_PREFIX_3 "blah-blah") calls. This will keep a lot of unnecc. info out of the messages log. 2. Removed some long, unnecc. comments. Just for the sake of cleaner code and logs. the privacy_update3.diffu is now obsolete and can also be removed. Thanks. By: Olle Johansson (oej) 2004-07-16 13:59:37 Does this still apply to cvs head? Any reason why it's hanging here? Does it need more testing? Are we ready for CVS? /Housekeeping By: Trevor Peirce (trev) 2004-07-16 14:06:01 I've been using this to screen calls, but none of it's database functionality to remember previous choices. For screening, this will be ready for prime-time once new voice prompts are recorded. For the database end, I can't say anything. By: Steve Murphy (murf) 2004-07-16 14:34:19 As far as I know, it's ready to roll, except for the sound files. Just in case, I've added a new diff against today's CVS. I've watched a few app_dial.c cvs updates go by, but all the diffs were in other areas, so there's not much difference in my diff's. You may differ about this, but, what's the diff? ;^) By: Mark Spencer (markster) 2004-07-16 22:44:54 Can we take the play_and_wait / play_and_record from app_voicemail and put it in app.c / app.h and then not have a copy in this patch? By: Steve Murphy (murf) 2004-07-18 23:26:18 Yes, this could be done. I've just compared the copied functions (play_and_wait(), and play_and_record() ), and while play_and_wait() hasn't changed at all, play_and_record() now has two new args in the voicemail version, fmt, and duration. I'm sure it would trivial to adapt the code to the new version. Oh, and one other possible problem. There's a global variable, silencethreshold, that's forced to 256 in the voicemail source, and I fix it at 128 in my code... is this going to be a problem? So, I propose this: move those functions as you propose. When it hits CVS, I'll modify the code to use the "public" versions, and test to make sure they still work right, and put up the new diff, OK? Or, do you want the honors? By: Mark Spencer (markster) 2004-08-07 18:34:25 Public versions would be good. I'd like this to be one of the first features to get added post 1.0 By: Steve Murphy (murf) 2004-08-08 16:08:15 I've been monitoring the CVS for a change to apps/voicemail.c and app.c to see when play_and_wait(), and play_and_record() are moved over. As soon as they are, I'll upgrade the app_dial.c code to deal with the new location. I don't know if you are intending to move these functions before or after 1.0, I guess it doesn't matter... By: Mark Spencer (markster) 2004-09-17 11:01:29 Okay finally there :) By: Steve Murphy (murf) 2004-09-17 18:39:12 Ok. The way you implemented the new funcs made the code fix easy. I like the way you took the global vars used in play_and_record, and put overrides in the func args. Nice. Anyway, I suggest waiting a few days before taking action. I'll test the new code over the weekend. Anyone else interested in this code are requested to thrash it thoroughly with the current cvs version.The tests I've done so far look like it's behaving exactly the same as it always has. By: Steve Murphy (murf) 2004-09-17 19:08:39 Updated README.privacy, aimed at the doc dir, with some more info/corrections. By: Steve Murphy (murf) 2004-09-30 20:42:57 OK, I've been testing both the databased and call screen options, and it looks good. Now, if we can get the Allison versions of the prompts, we are in business. By: twisted (twisted) 2004-10-27 16:04:10 do we have the prompts ready? Does this still work with current cvs? Need update to keep going. --Housekeeping By: Steve Murphy (murf) 2004-10-27 17:31:57 As best I can tell, no, the prompts are not ready... unless a new batch has come in from Allyson. And, yes, after I updated CVS, there was a conflict in app_dial.c with the new stuff. And, after resolving the conflict, there were a few errors in the compile. It looks like the callerid info in the channel structure has been modified. I updated the code. It compiles and links. I've updated this bug with a new patch (privacy_update7.diffu). It'll be a few days to test it properly. By: twisted (twisted) 2004-11-14 21:22:03.000-0600 Status, yet again? Prompts? Still work? --Housekeeping By: Steve Murphy (murf) 2004-11-23 08:32:32.000-0600 I've been testing it for weeks now, and it looks good. No word on the prompts. By: Mark Spencer (markster) 2004-12-11 20:29:34.000-0600 Do you already have a disclaimer? By: Steve Murphy (murf) 2004-12-12 08:35:01.000-0600 I remember faxing one in. If you cannot find it, I will gladly file another. By: twisted (twisted) 2005-01-04 00:58:43.000-0600 Was the disclaimer found? If not, was another filed? If so, is this ready? By: Steve Murphy (murf) 2005-01-04 14:16:46.000-0600 Actually, my perception (which is usually, but not always, faulty!) is that this enhancement is waiting for its sound prompts to be recorded by Allyson. I've sent the texts to someone who was supposedly gathering the next batch, but nothing's happened in a while. At least, no new batch has been done without the prompts for this update! They aren't on the wiki, I don't know what relationship there is between the person I sent the text to, vs. the wiki. I guess for safety, I could file the texts with the wiki...? By: Mark Spencer (markster) 2005-01-04 14:52:30.000-0600 If he sent in a disclaimer, we're fine (I'll take murf's word for it, I don't want to have to go search through the file if I can avoid it). We do need the prompts though and also need to see if this needs to be updated for latest CVS. By: robf (robf) 2005-01-04 18:41:21.000-0600 I'm getting ready to send a huge list of words & phrases to Allison, literally in the next couple of days. I took phrases for this patch from the Wiki, IIRC, but if you want to send the list directly to me (rob.fugina@gmail.com), I'll make sure I have the right ones... By: Steve Murphy (murf) 2005-01-05 14:54:01.000-0600 Saw another cvs update to app_dial.c, and uploaded a new patch. I remember pretty clearly filling out a disclaimer way back around a year ago, and faxing it in. It might not hurt to take everybody registered to bugs.digium.com, and have an attribute for disclaimer, which only someone at digium can set. As faxes come in, you can pair them up and set the bit. Heh, heh, you can even give a few points of karma for having the disclaimer option set... I've sent robf email with the prompts. By: robf (robf) 2005-01-08 11:58:49.000-0600 Attached tarball of requested sounds. By: Steve Murphy (murf) 2005-01-08 16:35:19.000-0600 I have uploaded a new version of priv-callpending.gsm that has some silence inserted at the beginning, about 1 second's worth. When I pick the phone, I consistently get the first second of this announcement chopped out. So, the silence allows the phone handset to reach the called party's ear from the cradle, before Allyson starts talking. The sounds are great! Wow! I also see that some significant (well... at least to me) changes were made to app_dial.c, that required some work and testing from me, namely the way some channel booleans are set and accessed. I believe I've modified my additions to follow the new regime. It seems to compile and seems to work as before. I will upload the new diffs here momentarily. By: Steve Murphy (murf) 2005-01-08 17:07:43.000-0600 PS. As to the production of the sound files: wow, that was quick! The diff-app_dial123 has been uploaded. By: Steve Murphy (murf) 2005-01-26 18:26:55.000-0600 Been using the screening option since last posting. Looks good. By: Steve Murphy (murf) 2005-02-01 22:13:26.000-0600 Just uploaded the diff-app_dial134 file, as I noticed that the play_and_record func has had an argument added, a lock file path to remove. Since care was already taken to name the introduction sound files that are recorded, so there will not be name collisions with other callers, the lock file is not necessary. I noticed that nothing is done if the arg is null, so I pass zero for this arg, and it seems to work just fine. By: Mark Spencer (markster) 2005-02-03 00:04:41.000-0600 murf, are you confident this is ready for merger, and if so, do we have a diff for sounds.txt and the sound files to be included? By: robf (robf) 2005-02-03 00:13:34.000-0600 Murf should confirm (and I know I was wrong on another bug), but I believe the prompts were included in the big asterisk-sounds submission last week... By: Steve Murphy (murf) 2005-02-03 09:33:50.000-0600 Yes, the sounds were included, and robf attached the file "privacy-sounds.tar" to this bug. I attached "priv-callpending.gsm", to override the supplied file in the tar file; I put a short stretch of silence at the beginning to allow the callee time to pick up the phone and put it to their ear before making the announcement. I just attached diff-sounds-txt, which is a patch for sounds.txt. Hmmm. There is one more thing. We need to, as part of the install process, to make sure we create the directory '/var/lib/asterisk/sounds/priv-callerintros', as the code doesn't create it... should I add a snippet of code to test for it, and create it, or should the asterisk "make install" do that? The code solution might fail due to permissions? Hmmm. By: Mark Spencer (markster) 2005-02-04 00:02:21.000-0600 Ooh, one thing caught my eye that bothers me... n+201 and n+301 can present problems because of layering one dial after another (e.g. two busies could put you at the same place as a single privacy and one more step). Can we use the DIALSTATUS to do this and create a new "stdextenwithprivacy" macro to show how we might handle that instead? By: Mark Spencer (markster) 2005-02-04 00:02:47.000-0600 As for the directory, I think "make install" is probably the right place for that. By: Steve Murphy (murf) 2005-02-04 22:04:20.000-0600 Sure. Just a Question or 2... I see that DIALSTATUS can take on the values: CHANUNAVAIL | CONGESTION | BUSY | NOANSWER | ANSWER | CANCEL So, I guess I'd add TORTURE and DONTCALL ? I'll look into a patch for the Makefile. By: Mark Spencer (markster) 2005-02-04 23:33:11.000-0600 Yes, I'm game with TORTURE and DONTCALL By: Steve Murphy (murf) 2005-02-05 12:03:40.000-0600 Just added "Makefile.diffu", which is a one-line patch to Makefile to make sure the sounds/priv-callerintros dir exists. I've just implemented the changes requested with ${DIALSTATUS}, but I'd like to test them a little before uploading the diff. Maybe later today. Will also upload some diffs to the docs to include the new possible DIALSTATUS codes. By: Mark Spencer (markster) 2005-02-05 16:19:47.000-0600 Okies and a sample macro (variation on macro-stdexten) would probably be good too! By: Steve Murphy (murf) 2005-02-05 17:16:42.000-0600 Gotcha. uploaded extensions.diffu, with a variant macro called stdPrivacyexten, using the 'p' option. And I just uploaded the diffs against app_dial.c for the current version, with the DIALSTATUS code present. Gave it a tryout here, works OK. By: Mark Spencer (markster) 2005-02-26 10:50:00.000-0600 Can you find me on IRC so we can go ahead and get this one in? I assume it's ready for merger but I want to be sure we get it all done. Thanks! By: Brian West (bkw918) 2005-03-17 21:16:02.000-0600 PLEASE FOR THE LOVE OF GOD CAN WE GET THIS IN CVS.. oh please. /b By: Brian West (bkw918) 2005-03-17 21:17:36.000-0600 OK YOU'RE KIDDING ME RIGHT... 14 month old patch? GIVE ME A BREAK GUYS.. lets get this in. Or something... JESUS.. hahaha /b By: Steve Murphy (murf) 2005-03-18 10:47:26.000-0600 Heh, heh! BKW is chomping at the bit, and after a year? If it's me you want to irc with, my IRC thing is saying: --- #asterisk :You need to be identified to join that channel and that's that. Does this mean I have to run an identd or something? I **used** to be be able to chat there... something is different. By: Michael Jerris (mikej) 2005-03-18 10:57:39.000-0600 murf- you need to register your nick /msg nickserv register <your-password> and identify on connects after that /msg nickserv identify <your-password> By: robf (robf) 2005-03-18 13:06:05.000-0600 /msg nickserv help By: Michael Jerris (mikej) 2005-05-02 08:02:06 Any luck on this murf? Post if you need a hand and we will get you in to IRC to get this one done. In the mean time, are there files that can be removed from this bug and do the patches still apply? By: Steve Murphy (murf) 2005-05-02 15:25:05 I just did a diff with the latest cvs-head version of app_dial.c, version 152, and uploaded it. You can remove: privacy_update4.diffu privacy_update5.diffu privacy_update6.diffu privacy_update7.diffu diff-app_dial119 diff-app_dial123 diff-app_dial134 diff-app_dial137 README.privacy (the first one, file_id 1976) The others files, etc. still look OK. The sound files were sent to me, should be available to you. I've tried IRC a few times, and never got on while Mark was around. Must be the name Murphy, or something. You can ring me on the telephone if you want to make communicating faster or more certain. my beltel num is 307-754-8154, ext 1. my Freeworld dialup # is 544716... Or, give me a time, I'll fire up another identd daemon and sign on. By: Michael Jerris (mikej) 2005-05-02 15:47:50 Cleaned up old files. This is ready for review please. By: Steve Murphy (murf) 2005-05-02 16:12:57 I'm on IRC right now, but ... unless someone who wants to talk to me in the next few minutes, I'll prob. just switch to another window and start checking on it occ., and may or may not spot someone trying to talk to me... I am not a power IRC user, and there's no sound on my workstation. IRC may not be the best way to "talk" to me. By: Kevin P. Fleming (kpfleming) 2005-05-15 20:47:49 OK, this looks ready to go... I'll review the bug notes and the code in a few hours, and try to get it merged. By: Steve Murphy (murf) 2005-05-16 07:59:40 I made a fix to the current app_dial.c, in the privacy code, to correct a problem just recently reported, where music-on-hold wasn't working when the target extension was a SIP phone. I recoded the test for moh to directly find the setting, rather than use the "tmp" structure, which seems to get modified. By: Steve Murphy (murf) 2005-05-16 08:02:47 Oh, and one more thing-- I recoded the new 'n' option in the patch to 'j'. I have been using 'N' and 'n' as a pair of options, and the new 'n' option breaks up the set. 'j' seemed a better intuitive option to me... but that's me. You can decide what to do there. By: Michael Jerris (mikej) 2005-05-23 23:09:23 Let's try one more time to get this guy in... Does the patch still apply? Can we just pick a new set of options instead of changing other already commited options? Files to cleanup? you know the drill... By: Steve Murphy (murf) 2005-05-23 23:41:31 Yep. I know the drill... ;^) OK, just made a patch for 153, then compared with the 152 ver. They are the same, a few line number diffs is all. You can delete the appdial-152.diffu. Don't get confused about the names, tho. It's the older patch. Yes, we could pick a new set of opts, but, this happened once before already, and I claimed squatter's rights, and Mark changed the 'n' to something else that time. That 'n' option is pretty popular. Anyway, you decide. I'm hopelessly biased. I'll agree with whatever you decide. I promise not to be offended in any way, shape, or degree. It's just a hassle changing the docs in and out of app_dial.c, if you get my drift. Oh, and make sure the priv-callpending.gsm overrides what's in the archive of sound files. Had to add an extra second of silence at the beginning, so as to allow the called party time to get the phone to their ear. That's all I can think of. Go for it...! By: Mark Spencer (markster) 2005-05-25 14:05:08 The ast_verbose() calls do not appear to be properly qualified with the right verbose options. There also appears to be a lot of "Dead code" that is ifdefed or commented out. Any chance of getting this cleaned up or are there special areas that need interest? By: Steve Murphy (murf) 2005-05-25 15:10:14 I looked at every ast_verbose() call, both in my code, and out, and then compared to how it is called in all the other apps, and I don't understand the problem. I've counted the args and %-constructs in the strings, and every call is OK. The code compiles and runs OK. Please, give me more info on this. There was one ifdef, I removed it, but it wasn't substantially large, and there were a few comments I knocked out, left as a part of the musing process while I wrote the code. I can upload what I've got, but it might be better if I understood the prob with ast_verbose first... By: Michael Jerris (mikej) 2005-05-25 16:34:11 Cleaned up sound files to avoid confusion. All the correct sound files are in 752sounds.tar. By: twisted (twisted) 2005-05-25 22:12:22 I think (and I hope i'm right here) that what markster means by unqualified verbose calls is not checking the current verbose option before we start writing verbose statements. ie: if (option_verbose > 2) ast_verbose(VERBOSE_PREFIX_3 "Verbose level 3 isn't printed unless we're verbose > 2"); This code also looks kinda noisy to me in the amount of verbose calls we make... are all of those calls necessary? shouldnt' some of that data be in ast_log(LOG_DEBUG, "blah "); instead? By: twisted (twisted) 2005-05-25 22:21:12 Formatting Fail - Lots of extreneous newlines, lots of non-uniform code layouts.. Some places, such as within the switch statement, we have lines like this: if (blah) { do_something(); } { do_something(); } which is kinda confusing. Formatting cleanup is also needed. By: Steve Murphy (murf) 2005-05-25 23:27:25 Excellent review, guys! Keep it coming! I'd rather have a single long list of things to do than a bunch of little ones spread over weeks. So, let's have that security, functionality, and architecture review! BTW, Are there extra karma points for passing all the reviews? ;^) By: Michael Jerris (mikej) 2005-05-25 23:33:58 Mark has pretty much approved the architecture on this one from his previous comments, and did a basic review of the code today. Twisted and I did a review and noted what we found. The only other note I had was to roll those diffu's into a single file, that will make commiting a lot easier. We will need some testers for a final test of this before it goes in. By: Steve Murphy (murf) 2005-05-26 14:33:02 Gentlemen-- I have uploaded privacy-omnibus.diffu, which is meant to replace these uploaded files: diff-sounds-txt Makefile.diffu README.variables.diffu extensions.diffu app_dial-152-diffu It doesn't replace the README.privacy; sorry. It has to be added to the repository anyway, best not to forget that. I fixed all ast_verboses (that remain) with a test. I debated using the log facility, but... logging lost. If you'd rather they were ast_log calls, let me know. The inserted blocks in the switch that twisted noted were removed. The only thing I can think of for the reason for the extra level existing might have been historical, as a temp var might have been declared and wrapped for locality of reference, but the var is gone, and with it the reason for the extra level of curlies. Sorry about that. Removed the old tests in the if() statements for moh/ringonly. I had that there for the last fix; but the person I did it for reported that the new code works, and it's ok to remove the old code. So I did. I *think* that covers every issue brought forward. I gave the code a quick test, so I know this new patch compiles and runs. Let me know if I missed something. It will be good to see this code find its place in the asterisk source! By: Michael Jerris (mikej) 2005-05-26 14:46:35 Ok we need one final set of code\formating review on this and most importantly TESTERS. There are a lot of people monitoring this so can we please have some testing and feedback in the bug so this one can finally go in. By: Michael Jerris (mikej) 2005-05-26 14:59:18 Around 1185 // comments are bad. Can we use ast_copy_string instead of strncpy. By: Steve Murphy (murf) 2005-05-26 15:48:56 OK, yes, I see the // comments, modified them. Also, checked the file for possible others, none exist. As to the strncpy->ast_copy_string, there are 28 such calls in the file. One is in privacy related code I haven't modified. 5 are in my code. The rest are scattered throughout the app_dial.c file. Do you want me to change them all, or just the ones in my code? By: Steve Murphy (murf) 2005-05-26 15:54:27 On second, third, and fourth thought: strncpy -> ast_copy_string : 1. do it as another bug report. 2. Makefile -Dstrncpy=ast_copy_string type substitution. 3. ? By: Michael Jerris (mikej) 2005-05-26 15:59:58 They are different in that you don't need to do the -1 on ast_copy_string, so you can't just replace it, and a little different in their behavior. ast_copy_string does not fill the rest of the string like strcpy does. I think we should use ast_copy_string where appropriate in code that is "added" in this bug, but leave the other ones in the file (ones previously in dial) as is so that we do not complicate this bug (it's been out here for long enough). By: Steve Murphy (murf) 2005-05-26 16:42:02 I have uploaded privacy-omnibus2.diffu, which supercedes the older privacy-omnibus file. I have modified the 5 strncpy calls to ast_copy_string calls. The third arg, if it had an x-1 value, has been modified to just x. and the // comments are now /* comments */. I compiled and ran a test before reforming the patch. All is still well. By: Steve Murphy (murf) 2005-06-09 23:19:05 Is there anything remaining on this issue for me to do? It seems to be pretty quiet, after a rush of activity... it's a year and half now... ? Is everybody busy, testing this out? By: Kevin P. Fleming (kpfleming) 2005-07-11 23:15:32 Committed to CVS HEAD, thanks! By: Digium Subversion (svnbot) 2008-01-15 15:07:45.000-0600 Repository: asterisk Revision: 3801 U trunk/app.c U trunk/apps/app_voicemail.c U trunk/include/asterisk/app.h ------------------------------------------------------------------------ r3801 | markster | 2008-01-15 15:07:44 -0600 (Tue, 15 Jan 2008) | 2 lines Move routines from voicemail to app for general use (part of bug ASTERISK-746) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=3801 By: Digium Subversion (svnbot) 2008-01-15 15:41:01.000-0600 Repository: asterisk Revision: 6102 U trunk/Makefile U trunk/apps/app_dial.c U trunk/configs/extensions.conf.sample A trunk/doc/README.privacy A trunk/sounds/priv-callee-options.gsm A trunk/sounds/priv-callpending.gsm A trunk/sounds/priv-introsaved.gsm A trunk/sounds/priv-recordintro.gsm A trunk/sounds/screen-callee-options.gsm U trunk/sounds.txt ------------------------------------------------------------------------ r6102 | kpfleming | 2008-01-15 15:41:00 -0600 (Tue, 15 Jan 2008) | 2 lines add privacy/screening functionality to app_dial (bug ASTERISK-746) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=6102 |