[Home]

Summary:ASTERISK-04649: [patch] chan_zap sets the numcomplete parameter to pri_sr_set_called backwards , so our numbers are never complete...
Reporter:Steve Davies . (stevedavies)Labels:
Date Opened:2005-07-21 14:44:26Date Closed:2011-06-07 14:10:03
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-addresscomplete.patch
Description:
We connected Asterisk to Hylafax via a PRI connection.  We found that Hylafax would not see the dialed number that we signalled through on the PRI.

Eventually after comparing PRI frames from our Telco to those that we were sending, we found that our dialled-number info did not have the number complete bit correctly.

In the end we traced this to the attached patch - chan_zap had the "backwards" sense for the number-complete bit, resulting in the number always being sent as not-complete.

(libpri, pri_sr_set_called is defined as: int pri_sr_set_called(struct pri_sr *sr, char *called, int calledplan, int numcomplete))

It almost never seems to matter that this wasn't right - but Hylafax does care.

The fix is confirmed correct by the fact that the information element now properly matches what the telco sends, and that Hylafax now sees our dialled number and routes the faxes properly.

Regards,
Steve Davies
Comments:By: Kevin P. Fleming (kpfleming) 2005-07-25 18:10:00

Committed to CVS HEAD, thanks!
By: Mark Spencer (markster) 2005-08-07 00:52:03

The original behavior is right.  This patch has been reverted.  To force "number complete" place a "w" at the end of the dial string (e.g. Dial(Zap/g2/${EXTEN}w))
By: Mark Spencer (markster) 2005-08-07 00:52:59

Reverted in CVS head.
By: Paul Cadach (pcadach) 2005-09-05 07:05:53

With 'w' at the end of dial string the condition ((s && *s) ? 1 : 0) at pri_sr_set_called() call is false, so 'w' wouldn't help. Following to comment on ASTERISK-2418, 'w' should not be specified for complete numbers. Also, "Sending Complete" IE should be ALWAYS used on links configured for enbloc dialing. So, IMHO condition should be changed to:
((s && p->pri->overlapdial) ? 0 : 1)

If full support for delayed dialing is required, there is 2 cases possible:
1) delayed dialing on enbloc link - rest of digits should be sent by DTMF after CONNECT signal received;
2) delayed dialing on overlap link - rest of digits should be sent by INFO messages in dialing stage of call.


By: Michael Jerris (mikej) 2005-09-05 08:31:43

Ok this is a duplicate of 1857, which one needs to be closed?
By: Steve Davies . (stevedavies) 2005-09-05 15:06:41

OK,

following up on this and picking up from the traces I posted on bug 1857.

Here is the trace when Jens dials with a trailing "w"

   -- Accepting call from '0832727727' to '2305' on channel 0/9, span 1
   -- Executing NoOp("Zap/9-1", "0832727727|2305") in new stack
   -- Executing Macro("Zap/9-1", "forward-pabx|2305|0832727727") in new stack
   -- Executing NoOp("Zap/9-1", "macro-forward-pabx=>2305:0832727727") in new stack
   -- Executing SetCallerPres("Zap/9-1", "allowed") in new stack
   -- Executing Dial("Zap/9-1", "Zap/r3/2305w||r") in new stack
   -- Requested transfer capability: 0x00 - SPEECH
amber*CLI>
> [ 02 01 4a 96 08 02 00 06 05 04 03 80 90 a3 18 03 a1 83 85 6c 0c 00 83 30 38 33 32 37 32 37 37 32 37 70 05 80 32 33 30 35 ]
amber*CLI>
> Informational frame:
> SAPI: 00  C/R: 1 EA: 0
>  TEI: 000        EA: 1
> N(S): 037   0: 0
> N(R): 075   P: 0
> 36 bytes of data
-- Restarting T203 counter
Stopping T_203 timer
Starting T_200 timer
> Protocol Discriminator: Q.931 (8)  len=36
> Call Ref: len= 2 (reference 6/0x6) (Originator)
> Message type: SETUP (5)
> [04 03 80 90 a3]
> Bearer Capability (len= 5) [ Ext: 1  Q.931 Std: 0  Info transfer capability: Speech (0)
>                              Ext: 1  Trans mode/rate: 64kbps, circuit-mode (16)
>                              Ext: 1  User information layer 1: A-Law (35)
> [18 03 a1 83 85]
> Channel ID (len= 5) [ Ext: 1  IntID: Implicit, PRI Spare: 0, Preferred Dchan: 0
>                        ChanSel: Reserved
>                       Ext: 1  Coding: 0   Number Specified   Channel Type: 3
>                       Ext: 1  Channel: 5 ]
> [6c 0c 00 83 30 38 33 32 37 32 37 37 32 37]
> Calling Number (len=14) [ Ext: 0  TON: Unknown Number Type (0)  NPI: Unknown Number Plan (0)
>                           Presentation: Presentation allowed of network provided number (3) '0832727727' ]
> [70 05 80 32 33 30 35]
> Called Number (len= 7) [ Ext: 1  TON: Unknown Number Type (0)  NPI: Unknown Number Plan (0) '2305' ]
   -- Called r3/2305w
amber*CLI>


Note the absence of the [a1] "Sending complete" flag.

I checked the customer's config and the span is set as overlapdial=yes.

Steve
By: Steve Davies . (stevedavies) 2005-09-05 15:36:10

OK - another attempt to persuade Mark that there is something wrong here...

In chan_zap.c line 1973, we call:

 pri_sr_set_called(sr, c + p->stripmsd + dp_strip, pridialplan,  (s && *s) ? 1 : 0);

the third parameter is "numcomplete".

So this call says: send sending-complete iff "s" points at something that isn't null.

Now, what's "s" all about?  Looking up in the code it seems to come from the code at line 1901.

Here the code searches for a "w" and leaves pointer "s" pointing to the character after the w (and replaces the w with a null terminator).

The intention seems pretty clear: if the dial string has 123w456, send the "123" in the initial setup, and then remember the 456 to be sent later.  So, logically, if we had digits after a w then the number is not complete.  If we don't, then it is.

Except that in the call to pri_sr_set_called, we set "numcomplete" to yes if there ARE more digits, and to no if there aren't.

Surely that's backwards.  Mark, explain why it's right as it is?

Except, maybe, if the digits after the w are not meant to be sent as part of the number, but rather we want the "application layer" at the remote end to see them.

If that's the intention, then the suggestion to use a trailing "w" to force numcomplete is still not right - because the test is for there to be something AFTER the w.

So - I guess the user either has to do Dial/gX/1234ww) or something, or the test in pri_sr_set_called needs to be changed to be something like:

 (s || !p->pri->overlapdial) ? 1 : 0

Reasoning: always number-complete if we aren't overlapdial
for overlapdial, we send number-complete if there was a "w" in the dialstring.

Well - that's different from what PCadach came up with; I guess the intention of the code actually isn't that clear...

Steve
By: Steve Davies . (stevedavies) 2005-09-06 02:57:42

Hmm.  Three notes from me in a row - embarrassing.

Anyway - Jens confirms that Dial(Zap/g3/1234ww) manages to get the number-complete to happen.

Not very intuitive but a work-around for the time being.

Steve
By: Paul Cadach (pcadach) 2005-09-06 03:27:45

Following to note 1 for table 3-15/Q.931, citate, "Included if the user or the network optionally indicates that all information necessary for call establishment is included in the SETUP message.". So, SENDING COMPLETE IE shouldn't be included on incomplete numbers. Enbloc mode doesn't allows to pass dialing information after SETUP message is sent, therefore incomplete numbers possible in overlap dialing mode only.

So, SENDING COMPLETE IE should NOT included when 'w' is specified (number is not complete, s != NULL) AND link configured for overlap dialing (p->pri->overlapdial != 0). Resulting condition is ((s && p->pri->overlapdial) ? 0 : 1),

Interaction between sites described in Q.931 spec at chapter 5.
Enbloc dialing:
caller         callee
SETUP ->
<- CALL PROCEEDING/ALERTING/CONNECT/DISCONNECT

Overlap dialing:
caller         callee
SETUP -> (without SENDING COMPLETE IE!!!)
<- SETUP ACKNOWLEDGE
INFORMATION ->
...
INFORMATION -> (with Sending complete indicator, either IE or '#' digit)
<- CALL PROCEEDING/ALERTING/CONNECT/DISCONNECT


By: Paul Cadach (pcadach) 2005-09-06 03:37:01

Dialing by 1234ww helps because of (s && *s) condition, which is true when and only when 'w' is specified at dial string and something else is placed after 'w' key-symbol. Anyway current handling of SENDING COMPLETE is wrong just because string '1234w111' should NOT add SENDING COMPLETE IE isn't have complete number (the same situation with '1234ww' - just second 'w' plays role of a digit).
By: Steve Davies . (stevedavies) 2005-09-06 15:20:02

Hi,

I think you and I were both misunderstanding the intended meaning and usage of the 123w456 dialling format.

I think I understand now that the intention is that the dialstring after the w is sent AFTER the remote end answers the call.  I offered this alternative understanding in my earlier rather rambling note on this bug.

Can someone confirm that this IS the intended meaning?

If that understanding is right, then in my example "123" IS the complete number (and so should be dialled with sending-complete signalled).  The 456 are simply digits to be DTMF/INFOed to the remote application after the call is connected.

One might argue that if 123w456 is dialled with 123 and sending-complete, then simple 123 should also be signalled as sending-complete.  However, there is (at least) the special case of Dial(Zap/g1) with no number at all, where the idea is that the trunk is picked up with no number dialled, and the entire number is sent as DTMF/INFO digits.  So I guess the reasoning is that simple Zap/g1/123 on an overlap span just sends the 123 and then further digits can be dialled from the originating device.

I can go along with that.

But the Dial(Zap/g1/123ww) is obviously not reasonable.

So - I'm still of the view that the right fix is simply to test if s points somewhere - if it does, there must have been a w and so the number is complete.

So pass one:  (s) ? 1 : 0

Now maybe on a non-overlap span the number should always be marked as complete.  I suspect it actually doesn't matter; but if we want that then the expression gets refined as

 (s || !p->pri->overlapdial) ? 1 : 0

as I suggested above.

Steve
By: Matthew Fredrickson (mattf) 2005-09-06 18:03:33

I believe that your interpretation is indeed correct.  It appears that the string specified after the 'w' character will be send after the call is answered.  I think our bug here is that the line is shown below.

                               if (strlen(s) > 1)
                                       snprintf(p->dop.dialstr, sizeof(p->dop.dialstr), "T%s", s);
                               else
                                       p->dop.dialstr[0] = '\0';
                               *s = '\0';
                               ^^^^^^^
                               s++;
                               ^^^^
It appears that our increment and assignment would invalidate our check here:

pri_sr_set_called(sr, c + p->stripmsd + dp_strip, pridialplan,  (s && *s) ? 1 : 0);

Basically, it looks like for this to work, we have to make sure that it sends complete if the above "else"
branch is followed.


By: Paul Cadach (pcadach) 2005-09-06 21:21:59

Matthew, is your note mean dial-strings without 'w' will not add 'SENDING COMPLETE' IE on all dialing type of spans? This is wrong for enbloc dialing mode.

In overlap mode there two cases for "delayed" dialing possible: "delayed" digits sends after SETUP ACK message (by INFO messages) or after ANSWER message (with DTMFs). Which case is correct?

Also, calling side could use overlap mode too and Asterisk should be capable to pass dialing information from calling side to called one. IMHO, there is a case for "stripped"/partial dial-strings like '12345w'. So, in this case "SENDING COMPLETE" IE should not be included in SETUP packet and both sides should be stay in dialing information collection stage until dialled side notifies about leaving this stage (by either PROCEEDING/ALERTING/CONNECT/DISCONNECT message).

So, Mark's comment on ASTERISK-2418 could be updated with "If none digits specified after 'w' option and span is configured for overlap dialing, the call being setup is switched to overlap dialing mode and subsequent dialing information received from calling side will be passed to called one". On other cases (if dial string isn't have 'w' option or anything specified after 'w' option) call will be made in enbloc mode (with insertion of "NUMBER COMPLETE" IE into SETUP message).

Is I'm correct?
By: Matthew Fredrickson (mattf) 2005-09-07 17:49:45

> Matthew, is your note mean dial-strings without 'w' will not add 'SENDING COMPLETE' IE on all dialing
> type of spans? This is wrong for enbloc dialing mode.

This is incorrect.  Sending 'SENDING COMPLETE' on an enbloc mode SETUP is entirely optional.  We just chose not to default to sending SENDING COMPLETE with non overlap calls.

> In overlap mode there two cases for "delayed" dialing possible: "delayed" digits sends after SETUP
> ACK message (by INFO messages) or after ANSWER message (with DTMFs). Which case is correct?

This flag allows the second method to be done, i.e. DTMFs after the ANSWER.  That is what this code was originally written to do.

> Also, calling side could use overlap mode too and Asterisk should be capable to pass dialing
> information from calling side to called one. IMHO, there is a case for "stripped"/partial dial-strings
> like '12345w'. So, in this case "SENDING COMPLETE" IE should not be included in SETUP packet and
> both sides should be stay in dialing information collection stage until dialled side notifies about
> leaving this stage (by either PROCEEDING/ALERTING/CONNECT/DISCONNECT message).

I agree with this.  The option to not send SENDING complete should be allowed "post dial".  However,
unless you made some changes that I don't know about, there doesn't appear to be a good way to do
overlap bridging in a way that could take advantage of this flag.  The digits currently are passed inband
after the initial setup message is sent.

> So, Mark's comment on 0002453 could be updated with "If none digits specified after 'w' option and
> span is configured for overlap dialing, the call being setup is switched to overlap dialing mode and
> subsequent dialing information received from calling side will be passed to called one". On other
> cases (if dial string isn't have 'w' option or anything specified after 'w' option) call will be made in
> enbloc mode (with insertion of "NUMBER COMPLETE" IE into SETUP message).

From what he just said, I don't think that this is the intention of the use of this particular flag. All it does is to force NUMBER_COMPLETE IE into the SETUP message.
By: Matthew Fredrickson (mattf) 2005-09-07 17:51:03

Ok, Mark just committed a fix for this.  Steve, can you update to latest HEAD and see if it's fixed?
By: Steve Davies . (stevedavies) 2005-09-09 03:20:21

Matt:

What fix is supposed to fix this issue?

1.503 of chan_zap.c removed the setting of p->call to NULL in line 2405 or so;
1.504 of chan_zap.c changes something to do with radio interface

I'm at a loss to see how either of these could have any effect on this problem??

Steve
By: Steve Hanselman (shanselman) 2005-09-09 06:43:52

I think the w flag needs to signal sending complete regardless.

Unless our goldstar sees sending complete it will ignore the called number and will not route DDI's, period.

The fix as was is to correct the order of the case and not to check for further digits, for me this works and I can send or not send calling complete as I wish by including or not including the w flag.
By: Matthew Fredrickson (mattf) 2005-09-09 12:09:03

I think that Mark forgot to commit it.  It should be in now.
By: Steve Davies . (stevedavies) 2005-09-10 05:33:04

Matt:

I see the change and I'm sure that will resolve the issue.

With this fix, here's the deal:

"Where you are dialling out on an overlap span, and want to explicitly indicate sending-complete to the remote system, you should add a "w" at the end of the dialled number.

For example:  Dial(Zap/g1/1234567w) with dial 1234567 in the SETUP frame with a sending-complete IE included"
By: Paul Cadach (pcadach) 2005-09-12 01:43:49

And I would like to vote not to use 'w' on enbloc span for indicating of sending complete. For example, H.323, etc. numbers is always "complete" in most implementations, so miss of 'w' at dialstring should allow easy switching between technologies (just by replacing technology prefix) and such dialstring would be always compatible with current span configuration. Usage of 'w' is allowed for peers with non-standard behaviour. Is I'm clear?
By: Matthew Fredrickson (mattf) 2005-09-12 14:38:40

I don't know if I'm following both of your thoughts; If this fixes the previously broken behavior of the 'w' flag, let's close this.  If you want to discuss how we can improve it, let's discuss this in IRC or in a feature request.
By: Matthew Fredrickson (mattf) 2005-09-13 22:38:50

fixed in head
By: Digium Subversion (svnbot) 2008-01-15 15:42:37.000-0600

Repository: asterisk
Revision: 6209

U   trunk/channels/chan_zap.c

------------------------------------------------------------------------
r6209 | kpfleming | 2008-01-15 15:42:36 -0600 (Tue, 15 Jan 2008) | 2 lines

set the 'number complete' bit properly (bug ASTERISK-4649)

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

http://svn.digium.com/view/asterisk?view=rev&revision=6209
By: Digium Subversion (svnbot) 2008-01-15 15:43:55.000-0600

Repository: asterisk
Revision: 6296

U   trunk/channels/chan_zap.c

------------------------------------------------------------------------
r6296 | markster | 2008-01-15 15:43:55 -0600 (Tue, 15 Jan 2008) | 2 lines

Revert improperly applied patch from bug ASTERISK-4649

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

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