Summary:	ASTERISK-05054: [patch] chan_zap.c not correctly handling polarity reversals caused by calling party hangups
Reporter:	Faris Raouf (faris1)	Labels:
Date Opened:	2005-09-11 19:16:25	Date Closed:	2008-01-15 15:49:15.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_zap.c.diff ( 1) chan_zap.c.modified.diff
Description:	The code recently added to chan_zap.c to detect and deal with polarity reversals, as mentioned in 0002847, ignores polarity reversals caused by the calling party hanging up when asterisk is in state AST_STATE_RING (as opposed to AST_STATE_RINGING or AST_STATE_UP). In addition, an extraneous else statement appears in the code. These two things result in polarity reversals signaling hangups not being detected on some UK installations ****** ADDITIONAL INFORMATION ****** I'm uploading a diff for HEAD as of 12th September 2005 with the modifications I have made to the polarity reversal code in chan_zap.c to resolve this issue. I could never have done this without the excellent work already done in 0002847. The first problem is that when an incoming call comes in and the line "rings", the state is AST_STATE_RING (on my UK British Telecom line, at any rate). The existing code was only hanging up if the state was AST_STATE_UP, which does not handle the situation where the caller hangs up before the call is processed. Similarly, although I can't test this because something other than a polarity switch appears to trigger the answering of the line, the section for answeronpolarityswitch ignores the call when in AST_STATE_RING - instead it is only looking for AST_STATE_RINGING (or DIALLING). The second problem is that there seems to be an else statement (between the answer section and the hangup section -- commented in my diff -- where I don't think there should be one. This results in the hangup code never being run in my situation. Could those people using polarity reversal to generate both hangups and answers please try the modified code with the "else" removed to see if it breaks anything? In my case the call is not answered by polarity reversal -- I'm not really sure what is triggering the answer -- so I can't test the case where you might have and use both answeronpolarityswitch=yes and hanguponpolarityswitch=yes My modification does not fully correct the situation where the calling party hangs up but the call is already being processed (specifically if Playback() is in use or the call is in a queue). For some reason up to 15 seconds elapse before the channel hangs up in this situation. And please be gentle on me  -- this is the very first issue I've ever reported, the very first diff I've ever created, and the very first time I've ever played around with someone's .c file! I've tried to read everything I can on this so I hope I've done all the right things, but if not.... Faris. p.s. Disclaimer fax on the way. Should be sent later on 12/09/05.
Comments:	By: Michael Jerris (mikej) 2005-09-11 20:57:49 Please comment the bug when the disclaimer has been sent in, thanks. By: Mark Spencer (markster) 2005-09-11 23:37:35 The polarity switch on answer *should* only activate on RINGING state because it's only relevant for outbound calls, not inbound calls.  I won't be able to actually review your patch until your disclaimer has been submitted, thanks! By: Faris Raouf (faris1) 2005-09-12 03:50:09 I was wondering about the onanswer part overnight actually -- the fact that it had AST_STATE_DIALLING in it had me thinking. The rest should stil be relevant. Disclaimer on the way any moment now. By: Faris Raouf (faris1) 2005-09-12 10:59:54 Disclaimer now faxed. By: Faris Raouf (faris1) 2005-09-12 11:09:15 I have modified my patch to remove the AST_STATE_RING that I incorrectly added to the answeronpolarityswitch=yes if statement. I have recompiled and tested, and is is still working as expected: When in AST_STATE_RING, and the calling party hangus up, chan_zap hangs up instantly. As far as I can tell, the else statement I removed really should not have been in there. Users of answeronpolarityswitch=yes and hanguponpolarityswitch=yes please test. The revised file is uploaded as chan_zap.c.modified.diff as I was not able to delete the original file. By: Kevin P. Fleming (kpfleming) 2005-09-25 23:18:00 Committed to CVS HEAD, thanks! By: Digium Subversion (svnbot) 2008-01-15 15:49:15.000-0600 Repository: asterisk Revision: 6661 U   trunk/channels/chan_zap.c ------------------------------------------------------------------------ r6661 | kpfleming | 2008-01-15 15:49:14 -0600 (Tue, 15 Jan 2008) | 2 lines handle polarity reversal hangup indication during an incoming call that is still ringing (issue ASTERISK-5054) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=6661