[asterisk-bugs] [Asterisk 0011688]: [patch] Allow Sipura handsets (SPA942) to use Asterisk SLA via implementation of Broadsoft Sip extensions

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
https://issues.asterisk.org/view.php?id=11688 
====================================================================== 
Reported By:                grossi
Assigned To:                dvossel
====================================================================== 
Project:                    Asterisk
Issue ID:                   11688
Category:                   Channels/chan_sip/NewFeature
Reproducibility:            N/A
Severity:                   feature
Priority:                   normal
Status:                     assigned
Asterisk Version:           1.4.16.2 
Regression:                 No 
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!): 96644 
Request Review:              
====================================================================== 
Date Submitted:             2008-01-05 10:58 CST
Last Modified:              2009-07-13 16:56 CDT
====================================================================== 
Summary:                    [patch] Allow Sipura handsets (SPA942) to use
Asterisk SLA via implementation of Broadsoft Sip extensions
Description: 
Modifications in chan_sip.c and pbx.h to accommidate rudimentary
implementation of Broadsoft SIP Access side extensions with the short term
goal of achieving SLA functionality for Sipura phones (tested on SPA942's)
. 

Currently I have full SLA compatability with SPA942's working as well as
integration with other (AASTRA 53i) SLA compatible phones on asterisk. 


====================================================================== 

---------------------------------------------------------------------- 
 (0107695) dvossel (administrator) - 2009-07-13 16:56
 https://issues.asterisk.org/view.php?id=11688#c107695 
---------------------------------------------------------------------- 
Thanks for the patch, I honestly don't have a great understanding of what
is actually going on, but I have a few comments on the code alone.

1. In transmit_state_notify() within the extension state switch case
	case AST_EXTENSION_REQLINESEIZE:
		add_header(&req, "Event" , "Line-Seize");
		add_header(&req, "Subscription-State", "active, 14");
Why is the Event header added here rather than in the CALL_INFO subscribed
switchcase below like all the others?

2. This may have been caused by later changes to the code, but in
transmit_state_notify() I see that tmp->string is added in the CALL_INFO:
switch case, add_header(&req, "Call-Info", tmp->str), and once again after
the switch case a few lines below, add_line(&req, tmp->str).  I don't
really understand why that should happen. 

3. There are still some coding guideline issues.
http://www.asterisk.org/developers/coding-guidelines.

Avoid unnecessary whitespace at the end of lines. 

if and else statements should look like this.
   if (blah) {
       stuff
   } else {
       stuff
   }

Make sure spacing is correct.
   if (blah){     should be     if (blah) {

Those are just a few examples.  Look over the guidelines, I'm sure there
are more I may have missed.

Thanks again for the contribution! 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2009-07-13 16:56 dvossel        Note Added: 0107695                          
======================================================================