Summary:	ASTERISK-10035: [patch] Restructure transmit_callstate function and calls to transmit_
Reporter:	Damien Wedhorn (wedhorn)	Labels:
Date Opened:	2007-08-07 06:50:27	Date Closed:	2011-06-07 14:07:25
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_skinny
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) transmit_a.diff ( 1) transmit_b.diff ( 2) transmit.diff ( 3) transmit2.diff ( 4) transmit3.diff ( 5) transmit3a.diff ( 6) transmit4.diff
Description:	Broke up transmit_callstate to it's individual components so now all transmit_ functions build a single packet (except transmit_response which actually sends the packet out). Additionally, modified all calls to transmit_ functions to pass device rather than session as the state of session is unknown and basically requires testing before every call. This is step 1 of a multiple step process in getting transfer into trunk. ****** ADDITIONAL INFORMATION ****** The breakup of the transmit_callstate allows for better reading of the code as it is easy to identify what packets are being transmitted and in what order. Additionally, it identifies situations where the packets transmitted do not make sense (see skinny_extensionstate_cb where it appears to be stopping non existent RTP). Another reason for the breakdown of transmit_callstate was to allow for the individual transmit_ items to be called from the hold/unhold stuff rather than building and sending packets in the subs themselves.
Comments:	By: Damien Wedhorn (wedhorn) 2007-08-07 15:53:11 Added transmit_a.diff which excludes the change of of sending d to transmit_ functions. Also added transmit_b.diff which is just the change to send d to transmit_ functions. By: Jason Parker (jparker) 2007-08-07 18:04:10 Just a note that transmit_b requires transmit_a (this is fine - just want people to be aware of it) By: sbisker (sbisker) 2007-08-20 08:09:03 Are all three patches required for this change?  Also, will this work for 1.4? By: Damien Wedhorn (wedhorn) 2007-08-20 16:02:35 sbisker - no. Only the first (transmit.diff). The other two are cummulative so if you apply transmit_a, and then transmit_b you will end up with the same result as if you just applied transmit.diff. The reason for the two extra is that part of the changes may go into 1.4 (transmit_a) while all of it would be applied to trunk. By: Michiel van Baak (mvanbaak) 2007-12-18 16:06:51.000-0600 ping! Any progress on getting this one committed ? By: Damien Wedhorn (wedhorn) 2007-12-19 04:11:55.000-0600 Added transmit2.diff. Updated to svn rev 93741. Tested and seems to work. I added a section (headed "Transmit response to session") in handle register message because it deals with sending a message to a session where a device does not exist. There will be a valid device for all other transmit_responses. By: Tilghman Lesher (tilghman) 2008-07-28 15:18:36 Is this simply waiting for a commit? By: Damien Wedhorn (wedhorn) 2008-07-29 19:40:42 There were basically two parts to this patch. The first was to pass device (rather than session) to the transmit_ stuff. The reason for this was that session can be dropped and we would need to test if their is a session before really doing anything (such as the logging bug mentioned elsewhere). The second was to restructure the transmit_ functions into singular transmits (some of the transmit_ functions sent multiple packets to the devices and where this wasn't appropriate the full transmit was coded in line). I think the first issue still exists but the second one was predominantly incorporated into the transfer revision (http://svn.digium.com/view/asterisk?view=rev&revision=125096), although there may be further adjustments. By: Michiel van Baak (mvanbaak) 2008-08-01 07:01:56 The second part is indeed merged with the transfer patch. By: Michiel van Baak (mvanbaak) 2008-08-01 16:56:07 last patch I uploaded (transmit3.diff) only has the device changes for current trunk. The last patch uploaded by wedhorn did not cover all the places in latest trunk. We cannot blame him, this one has been without love for way to long. By: Damien Wedhorn (wedhorn) 2008-08-01 19:59:32 new patch (transmit3a.diff) is transmit3.diff with minor changes in handle_register_message. Basic testing, seems to work. By: Digium Subversion (svnbot) 2008-08-02 07:21:14 Repository: asterisk Revision: 135300 U   trunk/channels/chan_skinny.c ------------------------------------------------------------------------ r135300 | mvanbaak | 2008-08-02 07:21:12 -0500 (Sat, 02 Aug 2008) | 8 lines pass device instead of session to transmit_ functions. (closes issue ASTERISK-10035) Reported by: wedhorn Patches:      transmit3a.diff uploaded by wedhorn (license 30) Tested by: wedhorn, mvanbaak ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=135300 By: Damien Wedhorn (wedhorn) 2008-08-02 16:39:01 Not quite complete. Segfaults when an unknown device attempts to connect. New patch transmit4.diff fixes. Tested with an unknown device attempting to register, doesn't crash. Edit: handle_register_message is the only place where we may transmit a response without have a valid device and that only occurs if a device attempts to register that isn't configured. I opted for adding transmit_response_bysession (which tranmit_response points to) but we could just write the transmit code in line in handle_register_message. Comments? By: Michiel van Baak (mvanbaak) 2008-08-02 18:15:18 13224 is the new bug for this new issue :) Can you please upload the patch there ? Thank you