[Home]

Summary:ASTERISK-17141: [patch] Sending out unnecessary PROCEEDING messages breaks overlap dialing
Reporter:Birger "WIMPy" Harzenetter (wimpy)Labels:
Date Opened:2010-12-21 02:30:59.000-0600Date Closed:2011-01-25 11:58:08.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_dahdi
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) bug18509.diff.txt
( 1) issue18509_cause_codes_trunk.patch
( 2) issue18509_early_media_v1.8_v2.patch
( 3) issue18509_early_media_v1.8_v3.patch
( 4) issue18509_early_media_v1.8.patch
( 5) wrongcause
Description:chan_dahdi sends out a PROCEEDING message as soon as a call has a match in the dialplan.
I think sending this message is unnecessary and can be harmful.
I think it should only be sent when Dial() is called with a definitive destination.

So far I thought chan_dahdi was unable to do outgoing overlap dialing, but I just found out that it does, but sabotages itself.

If you have an
exten => _N!,1,Dial(dahdi/g1/${EXTEN})
and use overlap dialing, the trouble is that you get a PROCEEDING after you dialed the first digit which will tell the phone that dialing has finished.


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

Some phones will allow you to send DTMF after receiving the message, but that is a) unreliable and b) the dialed number won't go in to your redial list beyond that point. (so in the  above example you end up with 1-digit numbers in your redial list)
Other phones won't allow sending DTMF before the CONNECT, making dialing a complete no go.


The situation gets even worse when the call fails. In that case it sends a PROGRESS with cause 2A "Switching equipment congestion" as it looks like, no matter what the received cause was.
a) If the call can't be set up it should send a DISCONNECT.
b) cause 2A would be appropriate when maxload, maxfiles or minmemfree get exceeded.
10 seconds later there is a DISCONNECT with cause 22 "no circuit or channel available".

Sometimes the received cause is retained.
I'm not sure, but I have the impression that may be the case if a PROGRESS has been received.

In band information is not forwarded. Not even in those 10 seconds.
Comments:By: Alec Davis (alecdavis) 2010-12-21 03:29:39.000-0600

bug18509.diff.txt
Only send Proceeding if a unique extension is found when starting PBX (dialplan), which allows for continuation of enblock+overlap dialout.

Untested.
Compiles only.
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-21 03:35:10.000-0600

Wow. That was quick.
But unfortunately it still sends PROCEEDING when entering the dialplan.
By: Alec Davis (alecdavis) 2010-12-21 03:49:19.000-0600

I may get a chance to debug on hardware tomorrow.
By: Alec Davis (alecdavis) 2010-12-21 13:35:08.000-0600

richard: whats the extension matching function that's going to match 'N!', as we need to suppress the 'Proceeding', to allow 'transparent' overlap dialling.
By: Richard Mudgett (rmudgett) 2010-12-21 14:42:47.000-0600

You need to examine the following functions:
ast_exists_extension()    nonzero if what you have matches
ast_canmatch_extension()  nonzero if what you have or could add matches
ast_matchmore_extension() nonzero if could add more digits to match

It is going to be a combination of those functions to get what you want:
if exists && !matchmore then send PROCEEDING.

I think issue ASTERISK-15594 caused this problem.
By: Alec Davis (alecdavis) 2010-12-21 15:04:32.000-0600

if exists && !matchmore then send PROCEEDING, is what the patch should achieve.
but I haven't debugged it yet.

re ASTERISK-1655789 umm yes, it was me.
By: Richard Mudgett (rmudgett) 2010-12-21 16:17:42.000-0600

The '!' dialplan matching character seems to have some inconsistency in its matching definition for the matching functions.  The ast_matchmore_extension() is treating the '!' as the end of a pattern and not considering that it is a zero or more match.

If the dialplan were instead changed to something like:
ignorepat => 9
exten => _9.,1,Dial(DAHDI/g1/${EXTEN:1})

And Alec's patch is used, it works.
By: Richard Mudgett (rmudgett) 2010-12-21 16:43:37.000-0600

wimpy: I want to see a packet capture of the PROGRESS with the cause code 2A for the phone and trunk spans.
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-21 17:37:39.000-0600

rmudgett: See 'wrongcause' for a trace.
Dialed 51666 from span 12 (CR 9) to span 11 as 666 (CR 5), which is invalid.


By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-21 18:04:52.000-0600

I think the trouble is not in the dialplan matching, but in the fact that a PROCEEDING is sent at all at that point.
I think optimally it should only be sent when either Dial has a definite peer or received a PROCEEDING itself, or when a playback with noanswer is started.
(In the later case I wouldn't see an issue if it had to be done manually via an explicit Proceeding() in the dialplan.)
In other cases an immediate CONNECT or DISCONNECT makes more sense.
By: Richard Mudgett (rmudgett) 2010-12-21 19:07:23.000-0600

wimpy: What was the dialplan executed to get the wrongcause trace?

I am failing to find why the PROGRESS message is being sent.  An AST_CONTROL_CONGESTION is the only thing that could send a PROGRESS message with that cause code and Dial() is not going to send that control frame.
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-21 19:16:33.000-0600

it's only a
exten => _5X!,1,Dial(dahdi/g1${EXTEN:1:1}/${EXTEN:2})
By: Richard Mudgett (rmudgett) 2010-12-21 20:32:18.000-0600

wimpy: I reproduced the PROGRESS congestion behavior.  That is expected behavior because the PBX ran off the end of the dialplan extension and the Dial() set the DIALSTATUS to CHANUNAVAIL.  If the extension had a Hangup() it would have worked as you expected.
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-21 22:11:54.000-0600

Indeed. Adding a Hangup() fixes the wrong cause thing.
Actually I don't even get a PROCEEDING any more either, but I guess that might be due to the patch by alecdavis.

Now I sit and wonder what's going on. I realised I didn't have autofallthroug=yes, but setting that does not make a change.

I have to admit I'm a little lost here.
Is that something that goes on inside chan_dahdi?

But still the cause values don't make sense.
By: Richard Mudgett (rmudgett) 2010-12-22 10:53:15.000-0600

Alec's patch works if you do not use the '!' wildcard.  If your dialplan pattern is now "_5." the patch will work.

The cause values make sense.
1) The PBX end dialplan processing examines the value of DIALSTATUS.  It sends an AST_CONTROL_CONGESTION to the channel driver when DIALSTATUS=CHANUNAVAIL.
2) Chan_dahdi sends a PROGRESS message with cause 42 (Switch congestion).  (A case could be made to change it to 34 (No channels available) since the causes have similar meaning).
3) After a 10 second timeout waiting for the channel driver to hangup, the PBX hangs up the call to the channel driver with cause 34.
By: Alec Davis (alecdavis) 2010-12-22 13:19:17.000-0600

richard:

1). Is it realistic to think that ast_matchmore_extension() might change?

2). An alternative is to be to inspect the extension match that's about to be executed, a function like ast_get_extension_match, thus in wimpy's example it would return '_5!' and then we can check for the '!' or '.' at the end, and suppress the Proceeding.
Note, there may already be a pointer to the current extension match, I just haven't found it yet.

The reason for both of the above is now with extension matching like '_5.' there is the timeout of 3 seconds.
By: Richard Mudgett (rmudgett) 2010-12-22 13:44:15.000-0600

Alec:
1) I don't know.  The matching code is rather complicated and I have not studied that code.

2) I don't think that will work as there could be multiple possible matches for a partial extension.  There is a scoring system to figure out the best match.

The timeout is necessary to see if any more digits are forthcoming.

I suppose another way to do this is to have two extensions: '5' and '_5.' to be equivalent to '_5!'.
By: Alec Davis (alecdavis) 2010-12-22 13:57:20.000-0600

My train of thought with checking for '!' or '.' where we send the Proceeding, is that the sig_pri code is about to execute the current match thats already been scored.
Or does the score get re-evaluated for every line in the dialplan as it's being executed?
By: Richard Mudgett (rmudgett) 2010-12-22 14:40:27.000-0600

It may get reevaluated because I have seen a bizarre dialplan like this:
exten => _9.,1,NoOp()
exten => 99,2,Noop()
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-22 16:01:52.000-0600

rmudgett:
Sure the situation won't happen if I cahnge '!' for '.',. but then overlap dialing can't be possible at all.

To the cause thing: Dial exits with HANGUPCAUSE=1 so why does it forward as cause 2A and 22? That does not fit at all.
Neither the Progress nor the cause in that case nor the cause 2A in general.

Is the dalay somehow related to the 'inbanddisconnect' parameter?
If I set that to yes, I can see it keeps the 2nd channel up, but no difference otherwise. No audio, either.
By: Richard Mudgett (rmudgett) 2010-12-22 23:36:29.000-0600

The issue18509_cause_codes_trunk.patch should clear up the cause code issue.
Chan_dahdi handling of the AST_CONTROL_CONGESTION was inconsistent with the cause codes.  It was responsible for the 42 and 34 cause codes.

The delay is done by the end of dialplan processing.  It is waiting 10 seconds for you to listen to the congestion tones being played and hang up the phone.
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-23 00:40:43.000-0600

Testing will probably have to wait til tonight.

Ok, so these 10s are for tones/announcements, but
1) it should be a DISCONNECT then, not a PROCEEDING and
2) it seems random to me. Sometimes I get a fast busy, sometimes silence and sometimes even a dialtone, but never the announcement received from the other channel. With inbanddisconnect=yes that is.
By: Birger "WIMPy" Harzenetter (wimpy) 2010-12-23 14:53:44.000-0600

Yes, thank you. That looks a lot better with the issue18509_cause_codes_trunk.patch.
No PROCEEDING and correct cause code.

Now it would be great if it would sound better, but maybe a new issue would be more appropriate.
By: Richard Mudgett (rmudgett) 2010-12-23 15:13:02.000-0600

The issue18509_cause_codes_trunk.patch does not do anything abut the PROCEEDING message.

Yes, open a new issue for the inband audio.  I almost have a patch ready to test for it.
By: Richard Mudgett (rmudgett) 2011-01-03 16:03:17.000-0600

The issue18509_early_media_v1.8.patch file incorporates the issue18509_cause_codes_trunk.patch file and allows outgoing overlap dialing to hear dialtone and other early media.

1) Combines several flags into a single enum value representing call progress level.

2) Allows multiple PROGRESS messages to be sent.

3) Adds better protection from sending out of sequence messages.

4) Added diagnostic messages for deferred overlap digits handling corner cases.
By: Richard Mudgett (rmudgett) 2011-01-03 16:04:43.000-0600

The issue18509_early_media_v1.8_v2.patch is a minor update.
By: Birger "WIMPy" Harzenetter (wimpy) 2011-01-14 17:07:08.000-0600

Sorry for the late reply.

I can confirm that I get an external dial tone with that patch, but I still can't continue to dial.
If I use a phone that allows to send DTMF in that state and continue that way I still get a call with no audio.
By: Richard Mudgett (rmudgett) 2011-01-17 17:00:10.000-0600

Alec:
I have been wondering if the best way to resolve the overlap dial problem is to revert ASTERISK-15594 and backport the trivial builtin Proceeding() application to v1.4.

wimpy:
The no audio problem may be the phone itself.  The bug17085.diff.txt patch in ASTERISK-15865 effectively removes the change made by ASTERISK-15594.
By: Richard Mudgett (rmudgett) 2011-01-17 17:37:04.000-0600

The issue18509_early_media_v1.8_v3.patch adds a compile conditional to remove the proceeding added by ASTERISK-15594 for testing purposes.
By: Alec Davis (alecdavis) 2011-01-18 04:13:44.000-0600

richard:

Providing ast function that can check for a '!' match seems more like the right thing to do. I've looked at that matching code, and well, yes it's complicated.

reverting ASTERISK-15594 we then will have dialplan writers needing to know when or not to send the proceeding.

But ultimately, the '!' was provided to allow for overlap dialing, so in the meantime, ASTERISK-15594 can be reverted.
By: Birger "WIMPy" Harzenetter (wimpy) 2011-01-18 14:34:57.000-0600

With the patch v3, dialing works perfectly.

As for the calls with no audio: That looks to not be related.
I can not get any audio at all with bridged calls ATM.
I will try to investigate in to the later one now.
By: Digium Subversion (svnbot) 2011-01-25 11:31:21.000-0600

Repository: asterisk
Revision: 303747

U   branches/1.4/main/pbx.c

------------------------------------------------------------------------
r303747 | rmudgett | 2011-01-25 11:31:19 -0600 (Tue, 25 Jan 2011) | 9 lines

Backport the Proceeding application.

Added in trunk -r129399.

Enable the workaround for issue ASTERISK-15865 and ASTERISK-17141.

(issue ASTERISK-15865)
(issue ASTERISK-17141)

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

http://svn.digium.com/view/asterisk?view=rev&revision=303747
By: Digium Subversion (svnbot) 2011-01-25 11:36:55.000-0600

Repository: asterisk
Revision: 303765

U   branches/1.4/channels/chan_dahdi.c

------------------------------------------------------------------------
r303765 | rmudgett | 2011-01-25 11:36:51 -0600 (Tue, 25 Jan 2011) | 40 lines

Sending out unnecessary PROCEEDING messages breaks overlap dialing.

Issue ASTERISK-15594 was a good idea.  Unfortunately, it breaks overlap dialing
through Asterisk.  There is not enough information available at this point
to know if dialing is complete.  The ast_exists_extension(),
ast_matchmore_extension(), and ast_canmatch_extension() calls are not
adequate to detect a dial through extension pattern of "_9!".

Workaround is to use the dialplan Proceeding() application early in
non-dial through extensions.

* Effectively revert issue ASTERISK-15594.

* Allow outgoing overlap dialing to hear dialtone and other early media.
A PROGRESS "inband-information is now available" message is now sent after
the SETUP_ACKNOWLEDGE message for non-digital calls.  An
AST_CONTROL_PROGRESS is now generated for incoming SETUP_ACKNOWLEDGE
messages for non-digital calls.

* Handling of the AST_CONTROL_CONGESTION in chan_dahdi/sig_pri was
inconsistent with the cause codes.

* Added better protection from sending out of sequence messages by
combining several flags into a single enum value representing call
progress level.

* Added diagnostic messages for deferred overlap digits handling corner
cases.

(closes issue ASTERISK-15865)
Reported by: shawkris

(closes issue ASTERISK-17141)
Reported by: wimpy
Patches:
     issue18509_early_media_v1.8_v3.patch uploaded by rmudgett (license 664)
     Expanded upon issue18509_early_media_v1.8_v3.patch to include analog
     and SS7 because of backporting requirements.
Tested by: wimpy, rmudgett

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

http://svn.digium.com/view/asterisk?view=rev&revision=303765
By: Digium Subversion (svnbot) 2011-01-25 11:42:45.000-0600

Repository: asterisk
Revision: 303769

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_dahdi.c

------------------------------------------------------------------------
r303769 | rmudgett | 2011-01-25 11:42:43 -0600 (Tue, 25 Jan 2011) | 47 lines

Merged revisions 303765 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r303765 | rmudgett | 2011-01-25 11:36:50 -0600 (Tue, 25 Jan 2011) | 40 lines
 
 Sending out unnecessary PROCEEDING messages breaks overlap dialing.
 
 Issue ASTERISK-15594 was a good idea.  Unfortunately, it breaks overlap dialing
 through Asterisk.  There is not enough information available at this point
 to know if dialing is complete.  The ast_exists_extension(),
 ast_matchmore_extension(), and ast_canmatch_extension() calls are not
 adequate to detect a dial through extension pattern of "_9!".
 
 Workaround is to use the dialplan Proceeding() application early in
 non-dial through extensions.
 
 * Effectively revert issue ASTERISK-15594.
 
 * Allow outgoing overlap dialing to hear dialtone and other early media.
 A PROGRESS "inband-information is now available" message is now sent after
 the SETUP_ACKNOWLEDGE message for non-digital calls.  An
 AST_CONTROL_PROGRESS is now generated for incoming SETUP_ACKNOWLEDGE
 messages for non-digital calls.
 
 * Handling of the AST_CONTROL_CONGESTION in chan_dahdi/sig_pri was
 inconsistent with the cause codes.
 
 * Added better protection from sending out of sequence messages by
 combining several flags into a single enum value representing call
 progress level.
 
 * Added diagnostic messages for deferred overlap digits handling corner
 cases.
 
 (closes issue ASTERISK-15865)
 Reported by: shawkris
 
 (closes issue ASTERISK-17141)
 Reported by: wimpy
 Patches:
       issue18509_early_media_v1.8_v3.patch uploaded by rmudgett (license 664)
       Expanded upon issue18509_early_media_v1.8_v3.patch to include analog
       and SS7 because of backporting requirements.
 Tested by: wimpy, rmudgett
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=303769
By: Digium Subversion (svnbot) 2011-01-25 11:49:24.000-0600

Repository: asterisk
Revision: 303771

_U  branches/1.8/
U   branches/1.8/channels/chan_dahdi.c
U   branches/1.8/channels/sig_pri.c
U   branches/1.8/channels/sig_pri.h
U   branches/1.8/channels/sig_ss7.c
U   branches/1.8/channels/sig_ss7.h

------------------------------------------------------------------------
r303771 | rmudgett | 2011-01-25 11:49:21 -0600 (Tue, 25 Jan 2011) | 54 lines

Merged revisions 303769 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r303769 | rmudgett | 2011-01-25 11:42:42 -0600 (Tue, 25 Jan 2011) | 47 lines
 
 Merged revisions 303765 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r303765 | rmudgett | 2011-01-25 11:36:50 -0600 (Tue, 25 Jan 2011) | 40 lines
   
   Sending out unnecessary PROCEEDING messages breaks overlap dialing.
   
   Issue ASTERISK-15594 was a good idea.  Unfortunately, it breaks overlap dialing
   through Asterisk.  There is not enough information available at this point
   to know if dialing is complete.  The ast_exists_extension(),
   ast_matchmore_extension(), and ast_canmatch_extension() calls are not
   adequate to detect a dial through extension pattern of "_9!".
   
   Workaround is to use the dialplan Proceeding() application early in
   non-dial through extensions.
   
   * Effectively revert issue ASTERISK-15594.
   
   * Allow outgoing overlap dialing to hear dialtone and other early media.
   A PROGRESS "inband-information is now available" message is now sent after
   the SETUP_ACKNOWLEDGE message for non-digital calls.  An
   AST_CONTROL_PROGRESS is now generated for incoming SETUP_ACKNOWLEDGE
   messages for non-digital calls.
   
   * Handling of the AST_CONTROL_CONGESTION in chan_dahdi/sig_pri was
   inconsistent with the cause codes.
   
   * Added better protection from sending out of sequence messages by
   combining several flags into a single enum value representing call
   progress level.
   
   * Added diagnostic messages for deferred overlap digits handling corner
   cases.
   
   (closes issue ASTERISK-15865)
   Reported by: shawkris
   
   (closes issue ASTERISK-17141)
   Reported by: wimpy
   Patches:
         issue18509_early_media_v1.8_v3.patch uploaded by rmudgett (license 664)
         Expanded upon issue18509_early_media_v1.8_v3.patch to include analog
         and SS7 because of backporting requirements.
   Tested by: wimpy, rmudgett
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=303771
By: Digium Subversion (svnbot) 2011-01-25 11:58:07.000-0600

Repository: asterisk
Revision: 303772

_U  trunk/
U   trunk/channels/chan_dahdi.c
U   trunk/channels/sig_pri.c
U   trunk/channels/sig_pri.h
U   trunk/channels/sig_ss7.c
U   trunk/channels/sig_ss7.h

------------------------------------------------------------------------
r303772 | rmudgett | 2011-01-25 11:58:01 -0600 (Tue, 25 Jan 2011) | 61 lines

Merged revisions 303771 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r303771 | rmudgett | 2011-01-25 11:49:20 -0600 (Tue, 25 Jan 2011) | 54 lines
 
 Merged revisions 303769 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r303769 | rmudgett | 2011-01-25 11:42:42 -0600 (Tue, 25 Jan 2011) | 47 lines
   
   Merged revisions 303765 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r303765 | rmudgett | 2011-01-25 11:36:50 -0600 (Tue, 25 Jan 2011) | 40 lines
     
     Sending out unnecessary PROCEEDING messages breaks overlap dialing.
     
     Issue ASTERISK-15594 was a good idea.  Unfortunately, it breaks overlap dialing
     through Asterisk.  There is not enough information available at this point
     to know if dialing is complete.  The ast_exists_extension(),
     ast_matchmore_extension(), and ast_canmatch_extension() calls are not
     adequate to detect a dial through extension pattern of "_9!".
     
     Workaround is to use the dialplan Proceeding() application early in
     non-dial through extensions.
     
     * Effectively revert issue ASTERISK-15594.
     
     * Allow outgoing overlap dialing to hear dialtone and other early media.
     A PROGRESS "inband-information is now available" message is now sent after
     the SETUP_ACKNOWLEDGE message for non-digital calls.  An
     AST_CONTROL_PROGRESS is now generated for incoming SETUP_ACKNOWLEDGE
     messages for non-digital calls.
     
     * Handling of the AST_CONTROL_CONGESTION in chan_dahdi/sig_pri was
     inconsistent with the cause codes.
     
     * Added better protection from sending out of sequence messages by
     combining several flags into a single enum value representing call
     progress level.
     
     * Added diagnostic messages for deferred overlap digits handling corner
     cases.
     
     (closes issue ASTERISK-15865)
     Reported by: shawkris
     
     (closes issue ASTERISK-17141)
     Reported by: wimpy
     Patches:
           issue18509_early_media_v1.8_v3.patch uploaded by rmudgett (license 664)
           Expanded upon issue18509_early_media_v1.8_v3.patch to include analog
           and SS7 because of backporting requirements.
     Tested by: wimpy, rmudgett
   ........
 ................
................

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

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