[Home]

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-0600Date Closed:2008-01-15 15:41:01.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents: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