[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:                     ready for review
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-08-11 18:13 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. 


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

---------------------------------------------------------------------- 
 (0108923) mmichelson (administrator) - 2009-08-11 18:13
 https://issues.asterisk.org/view.php?id=11688#c108923 
---------------------------------------------------------------------- 
dvossel asked me to take a look at this patch to give a review. I have a
few concerns about it. Note that I am not very familiar with the Broadsoft
SLA implementation, so if I say anything that is just plain wrong, I
apologize.

1. I think this has the potential to break overlap dialing. Specifically,
in handle_request_invite, if a Call-Info header is present, and an
extension was not matched in the dialplan, then a channel is allocated so
that a pbx may be started on the peer's notify_exten. The problem is that
gotdest will be equal to 1 if using overlap dialling. In such a case, we
should not make the assumption that the SLA capabilities are being used.

2. I find it troubling that the presence of a Call-Info header in an
incoming INVITE is enough to trigger the SLA behavior. RFC 3261 specifies
that the Call-Info header can have lots of information about the caller,
and of course RFC 3261 knows nothing of any SLA implementation. I
unfortunately don't know much about the Broadsoft SLA extensions, but I
would expect that the Call-Info header would have some specific value in it
to look for.

3. In handle_request_subscribe, you seem to have mixed presence and
call-info behavior. For instance, I don't know if this is possible, but
what if a Sipura were to send a SUBSCRIBE for event "presence?" The way the
patch is coded, just because the phone is a Sipura, it will be subscribed
to the call-info event instead. I think the handling of the call-info event
needs to be in a different if statement from the "presence" and "dialog"
handling.

4. There are some very minor coding guidelines violations, such as not
always placing a space after a comma and putting parentheses around the 0
in return (0); Since these are so minor, don't go out of your way to
correct them, but keep them in mind for the next time you submit a patch. 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2009-08-11 18:13 mmichelson     Note Added: 0108923                          
======================================================================