[Home]

Summary:ASTERISK-05823: [patch] 'r' option in app_dial not working correctly
Reporter:gkloepfer (gkloepfer)Labels:
Date Opened:2005-12-12 12:06:14.000-0600Date Closed:2011-06-07 14:10:45
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_dial
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:The 'r' option in app_dial is supposed to allow Asterisk to provide audiable ringing indication to the caller.  Currently app_dial provides audiable ringing indication regardless of the use of 'r'.

The patch to app_dial to correct this problem is attached.

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

Unfortunately I suspect many people have depended on the ringing sound being played to the caller without 'r' present.  Correcting this bug may present problems for a significant number of Asterisk users.

This has presented a problem for me because the in-band progress tones from the telco are mixing with the Asterisk-generated progress tones to cause a very odd-sounding ringing indication.  Using the Asterisk-generated progress tones with the 'r' option in this case (and muting the channel) is undesirable because doing this causes the telco-generated disconnect messages to not be played (and sound to the caller like the call is ringing and ringing).
Comments:By: Armin Schindler (armin) 2005-12-13 13:24:48.000-0600

I don't think your patch is correct. With your patch applied, the
caller-channel does not get the information about the called party is
ringing, which is sometimes needed.
Producing ringing sound and signaling the ringing event are two different
things.
By: gkloepfer (gkloepfer) 2005-12-13 13:33:15.000-0600

If that is correct, then there are other issues with the 'r' option
in other places in app_dial.c.
   
If you look where OPT_RINGBACK (what indicates that 'r' is provided) is
used, you'll notice that it is used to handle when ast_indicate() is
called.  Assuming what you're saying is correct, what is really needed
is an arg to ast_indicate() to tell whether to provide audiable feedback
to the caller.

Thanks for taking a look at what I did.  I do actually appreciate it.
Peer review is a good thing.
By: Kevin P. Fleming (kpfleming) 2005-12-13 17:01:28.000-0600

Can you please provide a complete console trace of Asterisk acting improperly, as the bug posting guidelines request? I suspect you have some sort of configuration issue, as ast_indicate can already decide when to send inband progress and when not to...
By: gkloepfer (gkloepfer) 2005-12-14 00:55:27.000-0600

Okay, I'm going to try to be as polite as I can about this.

Here is the test case you requested.  My extensions.conf file looks exactly like this:

------------------------------------
[default]

[OriginateInternal]
exten => 202,1,Dial(SIP/202)
------------------------------------

Pertinent parts of sip.conf are (204 is a Sipura ATA2000 and 202 is a Polycom IP phone): (passwords omitted)

------------------------------------
[202]
type=friend
secret=OMITTED
host=dynamic
nat=no
canreinvite=no
qualify=200
mailbox=200
callerid="Computer Room" <202>
disallow=all
allow=ulaw
context=OriginateInternal

[204]
type=friend
secret=OMITTED
host=dynamic
nat=no
canreinvite=no
qualify=200
mailbox=200
callerid=Kitchen Phone <204>
context=OriginateInternal
------------------------------------

Here is the trace on the console (-vvvvr)

------------------------------------
limbic# asterisk -vvvvvr
 == Parsing '/usr/local/etc/asterisk/asterisk.conf': Found
 == Parsing '/usr/local/etc/asterisk/extconfig.conf': Found
Asterisk SVN-branch-1.2-r7351M, Copyright (C) 1999 - 2005 Digium.
Written by Mark Spencer <markster@digium.com>
=========================================================================
Connected to Asterisk SVN-branch-1.2-r7351M currently running on limbic (pid = 91893)
Verbosity was 3 and is now 5
   -- Remote UNIX connection
limbic*CLI> extensions reload
 == Parsing '/usr/local/etc/asterisk/extensions.conf': Found
   -- Registered extension context 'default'
   -- Registered extension context 'OriginateInternal'
   -- Added extension '202' priority 1 to OriginateInternal
limbic*CLI>
limbic*CLI>
limbic*CLI>
   -- Executing Dial("SIP/204-4bae", "SIP/202") in new stack
   -- Called 202
   -- SIP/202-46c4 is ringing
 == Spawn extension (OriginateInternal, 202, 1) exited non-zero on 'SIP/204-4bae'
------------------------------------
(I called extension 202 from 204, heard the ringing, then hung up)

Guess what?  I'm going to tell you that the caller hears ringing indication even though I didn't spectify the 'r' option.  The console trace shows one command being executed, and that's Dial.

The reason why I knew this already is because it is essentially what I spent several hours doing with the complex production dialplan at the office after finally deciding it wasn't a hardware, configuration, or telco problem and looking at the source code.

Now, if I apply my patch, I'm going to execute the same test and show the console output:

---------------------------------
   -- Executing Dial("SIP/204-5874", "SIP/202") in new stack
   -- Called 202
   -- SIP/202-37ce is ringing
 == Spawn extension (OriginateInternal, 202, 1) exited non-zero on 'SIP/204-5874'
----------------------------------

With the new app_dial.so I heard no ringing indication.  Now, let me add the r option to Dial:

----------------------------------
limbic*CLI> extensions reload
 == Parsing '/usr/local/etc/asterisk/extensions.conf': Found
   -- Registered extension context 'default'
   -- Registered extension context 'OriginateInternal'
   -- Added extension '202' priority 1 to OriginateInternal
   -- Executing Dial("SIP/204-820b", "SIP/202||r") in new stack
   -- Called 202
   -- SIP/202-e525 is ringing
 == Spawn extension (OriginateInternal, 202, 1) exited non-zero on 'SIP/204-820b'
----------------------------------

and now I hear ringing indication, like I should.

I know what the bug reporting guidelines say.

Now while I realize that people submit a lot of bogus bug reports, I really do try to thoroughly go over every possible blunder on my part before I blame Asterisk and start looking at the source.  Unfortunately, Asterisk DOES have bugs, and it is through the efforts of people like myself who are trying to implement this in a production environment and who's employers are patient enough to allow their employees to debug the code that you get feedback from someone who has taken the time to understand the source enough to track something like this down.  This little problem is pretty darn subtle.  To provide all the extra information in this note took almost 2 hours of my own time on my own home system, in addition to all the previous work I did at the office today.  Quite frustrating.

Now please tell me if I have completely misunderstood what 'r' does in Dial, and if I completely misunderstand what the app_dial.c logic is supposed to be doing in the case I have demonstrated.

If that isn't enough, then I will further state that if you look at the code just before the patch I provided (the part I didn't change), you will also see that if one provides the 'm' option (provide music-on-hold to the caller during ring) that the exact same thing is being done to not call ast_indicate() (what my patch is doing when 'r' is not present).

As I said in my previous response, if the correct action here is to call ast_indicate() in both cases, then the logic in app_dial.c for both the 'm' and 'r' options in the remainder of the code is entirely wrong and needs to be rewritten, because that's not what's being done in the rest of the unpatched code.  If you don't believe me, then please look at it yourself.
By: twisted (twisted) 2005-12-14 12:44:09.000-0600

This is completely bogus.  The 'r' option in dial was to FORCE ringing regardless of progress on the remote end or not.  It was a hack, and a workaround.  What you're talking about doing is turning OFF progress of ringing to the sip devices unless the r option is enabled, which would be the entirely wrong behavior.

Please, before you jump to conclusions and jump down people's throats, find out what the 'r' option is really for.

     'r' -- indicate ringing to the calling party, pass no audio until answered.   <-- from "show application dial"  

If you read that, literally that means "indicate ringing regardless of actual progress, and do not allow audio to pass until(if) the call is answered".  

Put the r option in and call a busy device.  You will hear ringing, THEN busy.  Call a device that you've defined, but not hooked up.  You'll hear ringing, even though that device is not even reachable.  

Sorry, but this bug is bogus.
By: gkloepfer (gkloepfer) 2005-12-14 14:09:52.000-0600

1.  At what point do you see the word "force" in the documentation for the 'r' option in Dial?

2.  If I read that literally, it means EXACTLY what it says -- when 'r' is present indicate ringing to the calling party and pass no audio to same party until the call is answered.

3.  The action for when 'r' is absent is not well-defined in the literal reading of the documentation for app_dial, but from my understanding of the source and what I already know about how telephony works:  When a call is placed, an audio channel can be (and usually is) opened to the destination for in-band call progress information ("audio") to be passed (in the case of a disconnected number, it's the "you have reached a disconnected number" message).  Why?  Because if a call is placed to, for example, a disconnected number, the call is not actually completed (ie. nobody "answers").  If it were completed, you would be billed for calling disconnected numbers, which you aren't.  So the way *I* understand the absence of the 'r' option to Dial, it means when a call is placed, DO NOT indicate ringing to the calling party, and pass audio (the progress audio while ringing) to the calling party.

If you do not provide that audio pathway during ringing, or if you intercept it and replace it with something else, then the caller cannot hear the downstream intercept (disconnect, in this case) message.

4.  Who added the 'r' option?  Why did they add it and what was the intent?  Yes, I know you said it was a hack and a workaround, but I'd love to know what they were hacking or working around.

5.  If you really think the bug is bogus, close it.  Now I have a crystal clear understanding of why the OpenPBX fork happened.  I'm terribly sorry I wasted everyone's time (except for the two other people on -dev who felt they were experiencing similar issues).

PS: Before you jump too far down MY throat, please look at app_dial.c and tell everyone what happens to the ast_indicate in the same case when the 'm' option (it's OPT_MUSICBACK) is used.  That will turn off progress of ringing to SIP (or any other) devices when the 'm' option is provided.  Is that also incorrect?
By: twisted (twisted) 2005-12-14 14:29:34.000-0600

1) Nowhere, but as you also can obviously tell, we haven't, until recently by some 3rd parties, concise documentation.

2) if we tell something to indicate it, it does no matter what.  It's like turning on a light switch.

3) then it should have been obvious even by your understanding.  SIP is a bit different in most places, though.  We can have a 180 code or a 183, and the 183 opens the audio path early.  There is a thing called early media which means media can pass but the call has not been answered.  

4) It's been ages ago that was added, and I couldn't tell you by who, but at the time there was no progress being passed at all, and it was a hack to provide ringing even if we didn't get progress/ringing messages.

5) Okay, whatever.  I didn't close it to give you an opportunity to clarify.  

6) No.  the 'm' option is meant to REPLACE any progress messages we recieve.  It replaces it with music, and is operating as designed.

PS. on a side note:  Wouldn't you think it'd be silly to, by default, withold progress messages from the caller, and require the 'r' flag to allow them to recieve ringback?  

I read and understand what you're saying, but it's a simple misconception.    You asked to point out if you're misunderstanding, and that's what I did.
By: gkloepfer (gkloepfer) 2005-12-15 07:40:32.000-0600

| PS. on a side note:  Wouldn't you think it'd be silly to, by default,
| withold progress messages from the caller, and require the 'r' flag to
| allow them to recieve ringback?

Yes, I do think it's silly.  I didn't write Asterisk though.  I only try to maintain consistency with the rest of what's there.  That's why I was asking what the reason for 'r' actually was.  I honestly want to understand what was intended.

The issue here is not when using Dial from PBX-to-phone, but from PBX-to-PBX or from PBX-to-telco.  I included the phone example because it was easy to replicate.  The operation of Dial in that situation supports what I've been trying to say, particularly when you're talking to a Zap (PRI) or IAX2 channel, like I'm doing.  Two PBXen can be connected via SIP, obviously, as well, but I haven't been looking at that.

Look, I realize we have a difference of opinion here, and I'm obviously failing miserably at convincing you that there is a problem.  Let's agree to disagree, close the bug, decrement my karma, and get it over with.  I know that somewhere along the line this will rear its ugly head again, but this discourse is wasting both of our time (and maybe others' as well) and that is certainly not my intent.  I can easily patch the source for our installation and if something blows-up in my (or my users') face then I only have myself to blame.
By: gkloepfer (gkloepfer) 2005-12-15 15:00:34.000-0600

Something that twisted said in his last note caught my attention, and he's right.  This bug report is, indeed, bogus.  Please close and delete the patch so that someone doesn't actually consider it valid.

It would really help that if the workaround that 'r' was added for is no longer needed that it be removed so that there are fewer misunderstandings.

By the way, I am officially apologizing for all of this mess.  I have a few reasons for the way I reacted, but none are good excuses for the exchange that transpired.
By: twisted (twisted) 2005-12-15 16:34:10.000-0600

gklopfer:  Just so you know, I wasn't attempting to direct any negative attitude towards you, I apologize if it came off that way.  There won't be any karma hit, as you did make us quie aware that there is obviously an issue with how the 'r' option is percieved.  feel free to open up a new bugnote if you want to write a patch to either have the option removed, or perhaps we should make the option do something else, such as 'r'estrict progress (why someone would want to do that is beyond me, but they could), or even 'r'eplace progress with another type, since we basically do that now, but don't let the oper choose what to replace it with.
By: twisted (twisted) 2005-12-15 16:34:32.000-0600

closed per reporter