[Home]

Summary:ASTERISK-15849: [patch] chan_dahdi/fxs really needringing
Reporter:Tzafrir Cohen (tzafrir)Labels:
Date Opened:2010-03-21 05:33:19Date Closed:2010-06-22 16:06:53
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_dahdi
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) needringing_debugmessages.diff
( 1) needringing.diff
Description:Symptom: an analog phone connected to an FXS device (see more below on the FXS device). Asterisk rings to the phone. If the call is answered before the end of the first ring, the channel is in state Ringing rather than in state Up.

(needringing is one of the boolean flags included in the pvt struct of chan_dahdi).

Details:
Our DAHDI driver may delay sending PCM until the first ring. This seems to trigger a strange bug in DAHDI where the driver apparently remains in state Ringing rather than switching to Up if the phone was picked up before the first ring.

Looking deeper into this we see that the state is set to 'Up' but right afterward set back to Ringing.


In the following cases I generated a call to the phone and picked it up before the end of the first ring. Line numbers are affected by the heavy patching for extra debug messages. See attached patches.

Looking at how needsringing is set and used, I see that the state is set to RINGING 3 times:

1. Right after setting needringing to 1 in dahdi_call().
2. When needringing is "consumed" in dahdi_read(), the state is set to RINGING.
3. And in the same place a control frame is put to set the state to RINGING.

(3) is the one that cases the delay setting of state which triggers (triggers? causes?) the bug.

However as I assume that there's something here I don't fully understand, I tried the simplest fix: unset needringing if we already answered.

Without the fix:
morgan*CLI> originate DAHDI/4 application echo
[Mar 21 12:11:42] DEBUG[21563]: chan_dahdi.c:10977 dahdi_request: Using channel 4
[Mar 21 12:11:44] DEBUG[21563]: chan_dahdi.c:6702 __dahdi_exception: Exception on 18, channel 4
[Mar 21 12:11:44] DEBUG[21563]: chan_dahdi.c:5735 dahdi_handle_event: Got event Ring/Answered(2) on channel 4 (index 0)
[Mar 21 12:11:44] DEBUG[21563]: chan_dahdi.c:2686 dahdi_enable_ec: Enabled echo cancellation on channel 4
[Mar 21 12:11:44] DEBUG[21563]: chan_dahdi.c:2705 dahdi_train_ec: No echo training requested
[Mar 21 12:11:44] DEBUG[21563]: chan_dahdi.c:6077 dahdi_handle_event: channel 4 answered
   -- Launching echo() on DAHDI/4-1
[Mar 21 12:11:44] DEBUG[22602]: chan_dahdi.c:6801 dahdi_read: subchan needringing: adding frame RINGING
[Mar 21 12:11:44] DEBUG[22602]: channel.c:3474 ast_write: ast_write: chan DAHDI/4-1, indication 3
[Mar 21 12:11:44] DEBUG[22602]: chan_dahdi.c:7185 dahdi_indicate: Requested indication 3 on channel DAHDI/4-1


A trace after the fix:
morgan*CLI> originate DAHDI/4 application echo
[Mar 21 11:54:35] DEBUG[21563]: chan_dahdi.c:10977 dahdi_request: Using channel 4
[Mar 21 11:54:37] DEBUG[21563]: chan_dahdi.c:6702 __dahdi_exception: Exception on 18, channel 4
[Mar 21 11:54:37] DEBUG[21563]: chan_dahdi.c:5735 dahdi_handle_event: Got event Ring/Answered(2) on channel 4 (index 0)
[Mar 21 11:54:37] DEBUG[21563]: chan_dahdi.c:2686 dahdi_enable_ec: Enabled echo cancellation on channel 4
[Mar 21 11:54:37] DEBUG[21563]: chan_dahdi.c:2705 dahdi_train_ec: No echo training requested
[Mar 21 11:54:37] DEBUG[21563]: chan_dahdi.c:6077 dahdi_handle_event: channel 4 answered
   -- Launching echo() on DAHDI/4-1
morgan*CLI>

(Initially tested on 1.6.2 as it is what we work with and the code in trunk is quite reshuffled. trunk seems to have the same symptoms.

Patches:
* needsringing.diff: the fix I mentioned. vs. 1.6.2. Older versions should be similar.
* needsringing_debugmessages.diff: extra debug messages I added to trace the issue.
Comments:By: Digium Subversion (svnbot) 2010-04-30 17:22:46

Repository: asterisk
Revision: 260434

U   branches/1.4/channels/chan_dahdi.c

------------------------------------------------------------------------
r260434 | jpeeler | 2010-04-30 17:22:46 -0500 (Fri, 30 Apr 2010) | 11 lines

Ensure channel state is not incorrectly set in the case of a very early answer.

The needringing bit was being read in dahdi_read after answering thereby
setting the state to ringing from up. This clears needringing upon answering
so that is no longer possible.

(closes issue ASTERISK-15849)
Reported by: tzafrir
Patches:
     needringing.diff uploaded by tzafrir (license 46)

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

http://svn.digium.com/view/asterisk?view=rev&revision=260434
By: Digium Subversion (svnbot) 2010-04-30 17:36:49

Repository: asterisk
Revision: 260437

_U  trunk/
U   trunk/channels/chan_dahdi.c
U   trunk/channels/sig_analog.c
U   trunk/channels/sig_analog.h

------------------------------------------------------------------------
r260437 | jpeeler | 2010-04-30 17:36:49 -0500 (Fri, 30 Apr 2010) | 18 lines

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

........
 r260434 | jpeeler | 2010-04-30 17:22:46 -0500 (Fri, 30 Apr 2010) | 11 lines
 
 Ensure channel state is not incorrectly set in the case of a very early answer.
 
 The needringing bit was being read in dahdi_read after answering thereby
 setting the state to ringing from up. This clears needringing upon answering
 so that is no longer possible.
 
 (closes issue ASTERISK-15849)
 Reported by: tzafrir
 Patches:
       needringing.diff uploaded by tzafrir (license 46)
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=260437
By: Digium Subversion (svnbot) 2010-04-30 17:45:57

Repository: asterisk
Revision: 260439

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

------------------------------------------------------------------------
r260439 | jpeeler | 2010-04-30 17:45:56 -0500 (Fri, 30 Apr 2010) | 25 lines

Merged revisions 260437 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r260437 | jpeeler | 2010-04-30 17:36:49 -0500 (Fri, 30 Apr 2010) | 18 lines
 
 Merged revisions 260434 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r260434 | jpeeler | 2010-04-30 17:22:46 -0500 (Fri, 30 Apr 2010) | 11 lines
   
   Ensure channel state is not incorrectly set in the case of a very early answer.
   
   The needringing bit was being read in dahdi_read after answering thereby
   setting the state to ringing from up. This clears needringing upon answering
   so that is no longer possible.
   
   (closes issue ASTERISK-15849)
   Reported by: tzafrir
   Patches:
         needringing.diff uploaded by tzafrir (license 46)
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=260439
By: Digium Subversion (svnbot) 2010-04-30 17:47:26

Repository: asterisk
Revision: 260440

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

------------------------------------------------------------------------
r260440 | jpeeler | 2010-04-30 17:47:26 -0500 (Fri, 30 Apr 2010) | 25 lines

Merged revisions 260437 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r260437 | jpeeler | 2010-04-30 17:36:49 -0500 (Fri, 30 Apr 2010) | 18 lines
 
 Merged revisions 260434 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r260434 | jpeeler | 2010-04-30 17:22:46 -0500 (Fri, 30 Apr 2010) | 11 lines
   
   Ensure channel state is not incorrectly set in the case of a very early answer.
   
   The needringing bit was being read in dahdi_read after answering thereby
   setting the state to ringing from up. This clears needringing upon answering
   so that is no longer possible.
   
   (closes issue ASTERISK-15849)
   Reported by: tzafrir
   Patches:
         needringing.diff uploaded by tzafrir (license 46)
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=260440
By: Digium Subversion (svnbot) 2010-04-30 17:48:09

Repository: asterisk
Revision: 260441

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

------------------------------------------------------------------------
r260441 | jpeeler | 2010-04-30 17:48:09 -0500 (Fri, 30 Apr 2010) | 25 lines

Merged revisions 260437 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r260437 | jpeeler | 2010-04-30 17:36:49 -0500 (Fri, 30 Apr 2010) | 18 lines
 
 Merged revisions 260434 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r260434 | jpeeler | 2010-04-30 17:22:46 -0500 (Fri, 30 Apr 2010) | 11 lines
   
   Ensure channel state is not incorrectly set in the case of a very early answer.
   
   The needringing bit was being read in dahdi_read after answering thereby
   setting the state to ringing from up. This clears needringing upon answering
   so that is no longer possible.
   
   (closes issue ASTERISK-15849)
   Reported by: tzafrir
   Patches:
         needringing.diff uploaded by tzafrir (license 46)
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=260441
By: Digium Subversion (svnbot) 2010-06-14 19:41:32

Repository: asterisk
Revision: 270404

U   branches/1.4/channels/chan_dahdi.c

------------------------------------------------------------------------
r270404 | alecdavis | 2010-06-14 19:16:03 -0500 (Mon, 14 Jun 2010) | 7 lines

fixes FXS port still ringing when answered, as reported by Tzafrir on dev-list.

(issue ASTERISK-15849)
Reported by: tzafrir
Tested by: alecdavis


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

http://svn.digium.com/view/asterisk?view=rev&revision=270404
By: Digium Subversion (svnbot) 2010-06-22 16:06:53

Repository: asterisk
Revision: 271979

_U  tags/1.4.33.1/
U   tags/1.4.33.1/channels/chan_dahdi.c

------------------------------------------------------------------------
r271979 | russell | 2010-06-22 16:06:52 -0500 (Tue, 22 Jun 2010) | 8 lines

Merge revision 270404 from the 1.4 branch.

fixes FXS port still ringing when answered, as reported by Tzafrir on dev-list.

(issue ASTERISK-15849)
Reported by: tzafrir
Tested by: alecdavis

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

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