Summary:	ASTERISK-12379: [PATCH] duplicate code for setting LSPI
Reporter:	Mark Wilkinson (markwilkinson)	Labels:	patch
Date Opened:	2008-07-15 03:38:43	Date Closed:	2018-01-02 08:49:50.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_dahdi/NewFeature Channels/chan_dahdi/SS7
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) dahdi-dup_code.patch ( 1) dahdi-dup_code-v2.patch
Description:	After talking with Matthew and Alan via E-mail, I noticed in chan_dahdi.c there are 2 near identical calls to set the SS7 LSPI parameters dependant on channel variables SS7_RLT_ON and SS7_LSPI_IDENT code begins around line 2455 in chan_dahdi.c ****** ADDITIONAL INFORMATION ****** I have a patch, which removes the duplicate code, and alters the channel variable slightly to allow the setting of not just the LSPI Ident, but the Type, Scheme & Context - This may be of use for getting the RLT to work with PBX's other than Nortel.
Comments:	By: Paul Belanger (pabelanger) 2008-07-15 08:30:56 Did you forget your patch? By: Mark Wilkinson (markwilkinson) 2008-07-15 13:12:08 See attached file for patch By: Eliel Sardanons (eliel) 2008-07-15 15:07:50 Please remove the spaces inside the if  statement after "(", if (   something   ) { }, I only see this in one place. Thanks for the patch By: Mark Wilkinson (markwilkinson) 2008-07-15 15:37:06 Have altered the patch file to remove the spaces around the if statements as requested. New patch file is dahdi-dup_code-v2.patch By: Alan McMillan (alanmcmillan) 2008-07-17 14:05:14 Mark, Matt- I have a concern about combining the SS7_LSPI_IDENT parm and the SS7_RLT_ON parm.. The SS7_LSPI_IDENT parm is an optional parameter in and IAM and I do not know if it being set means that we are going to want to use RLT process. It just so happens that when it is set to the values I hard coded in CHAN_DAHDI(on the BLEG IAM) it causes the NORTEL DMS-250 to send back the CALLREFERNCE values needed to identify the leg of the call so the FAR can being sent on the ALEG bridges successfully to the correct BLEG and drops the ASTERISK out of the call. Also my SEND_FAR is not complete. The current version of RLT in asterisk only allows for a NON-ANSWERED call to be successfully RLT’d off the ASTERISK.  The SEND_FAR was being developed for ANSWERED RLT types. So as of today the dialplan needs to be as follows to allow RLT on the NORTEL to work properly.   exten => _X.,1,Set(_SS7_LSPI_IDENT=206555) exten => _X.,n,Set(_SS7_RLT_ON=YES) exten => _X.,n,Dial(ZAP/g1/8005551212,30) exten => _X.,n,Hangup(16); I think some of Marks patch should be added allowing the SS7_LSPI_IDENT to be fully set from the dialplan. But using that to trigger the RLT process needs to be researched a little more. If mark want to develop further the RLT code that also cool.  Either way its nice to have others looking into and giving feedback.. Alan By: Leif Madsen (lmadsen) 2008-12-05 08:53:03.000-0600 I'm curious if Mark would like to look into the issues that Alan has raised and move this forward, or if we would prefer to leave the code as is? Thanks! By: Tilghman Lesher (tilghman) 2009-02-26 12:19:48.000-0600 markwilkinson:  Could you address the concerns given by alanmcmillan please? By: Mark Wilkinson (markwilkinson) 2009-03-02 02:35:40.000-0600 You'll have to excuse me a moment while I get back into what I was thinking with this patch... it's been a little while since I've had time to look at it. From what I remember, looking through the original code, there are 2 calls to isup_set_lspi following each other. The first is called if variable SS7_LSPI_IDENT is set. The second is called if SS7_RLT_ON is set and is not set to NO. In both cases, the call parameters are the value of the channel variable, then the magic nortel numbers of 0x18, 0x7, 0x00. From reading the code and the e-mail I received from Alan/Matthew (on/around 14th July 2008 - stating that to use the feature, in dialplan set SS7_RLT_ON=<ident>, it seemed to me that both pieces of code did exactly the same function. i.e. You could set SS7_LSPI_IDENT=<ident> and not set SS7_RLT_ON or have SS7_RLT_ON set to NO and the feature would still work. It was for this reason that I initially raised the patch. From alan's note above, setting SS7_RLT_ON to yes, would cause the message to be altered (changing the ident in the FAR to be set to YES) By: Corey Farrell (coreyfarrell) 2017-12-14 13:55:33.477-0600 Are you interested in pursuing this further?  If so the patch will need to be rebased so it can apply the the current {{master}} branch and it will need to go through code review. ---- Thanks for the contribution! If you'd like your contribution to be included faster, you should submit your patch for code review by the Asterisk Developer Community. To do so, please follow the Code Review \[1\] instructions on the wiki. Be sure to: * Verify that your patch conforms to the Coding Guidelines \[2\] * Review the Code Review Checklist \[3\] for common items reviewers will look for * If necessary, provide tests for the Asterisk Test Suite that verify the correctness of your patch \[4\] When ready, submit your patch and any tests to Gerrit \[5\] for code review. Thanks! \[1\] https://wiki.asterisk.org/wiki/display/AST/Code+Review \[2\] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines \[3\] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist \[4\] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Documentation \[5\] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage By: Asterisk Team (asteriskteam) 2018-01-02 08:49:50.313-0600 Suspended due to lack of activity. This issue will be automatically re-opened if the reporter posts a comment. If you are not the reporter and would like this re-opened please create a new issue instead. If the new issue is related to this one a link will be created during the triage process. Further information on issue tracker usage can be found in the Asterisk Issue Guidlines [1]. [1] https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines