[asterisk-dev] [Code Review]: Changes from Mantis ID 13495 in trunk.

Mon Apr 23 10:05:58 CDT 2012

> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.h, lines 221-228
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23473#file23473line221>
> >
> >     This redirecting information should also be put into the ast_channel.redirecting.from structure.  The redirecting information is accessed by the REDIRECTING dialplan function.  Other channel technologies (SIP and ISDN) use the ast_channel.redirecting information instead of channel variables.
> >     
> >     Outgoing calls should prefer the data in the ast_channel.redirecting.from structure instead of the SS7 channel technology specific channel variable.
> 
> KNK wrote:
>     I am having some problems with this change:
>     1. REDIRECTING dialplan function sets ast_channel.redirecting.from and ast_channel.redirecting.to only, while for SS7 there is also the original from. Should the function be extended with orig_from ast_party_id and orig_reason or we should store only the last from and use SS7 specific variables for the orig_*
>     2. There are two redirect counters and info_indication, which should also go in ast_party_redirecting?
>     3. How the indication info should be translated from/to XXX-num-pres?
>     
>     I think we should store only the from and to numbers for the other channels (if redirection information presentation is not restricted) and presentation to allowed/restricted (from indication info), use the highest redirect counter (they should be equal), but keep the specific variables.
>     For outgoing calls: parse redirecting data only if SS7_REDIRECT_INFO_IND is defined and also keep SS7_REDIRECT_INFO_ORIG_REAS specific variable. If SS7_REDIRECT_INFO_IND is set to allowed it may be changed to restricted based on the from/to presentation or force the provided value.
> 
> KNK wrote:
>     Review 1829 adds orig_* to REDIRECTING(), so they should go there as i understand.
>     
>     According to http://tools.ietf.org/html/draft-levy-sip-diversion-11#page-39 if i understand it properly the redirect_info_ind should be set from the orig-num-pres (presentation allowed/restricted) and probably from the to-num-pres (when set to restricted) we can set the indication to XXX_INFO_RESTRICTED.
>     
>     The CALL_REROUTED_ / CALL_DIVERTED_ part comes from the reason field, so there is no need of separate field for redirecting info indication, but to keep the possibility to override it from the dialplan i will keep that variable for both in/out.
>     
>     For the outgoing calls i am planning to set both redirect counters from the same value (ast_party_redirecting.count) and for the incoming to store the highest one, but issue a warning if they are different.
>     
>     Is this the correct approach?
> 
> rmudgett wrote:
>     I added the original redirecting party to REDIRECTING because ISDN Q.SIG and ETSI PTP also supply and use that information.  The ISDN code was already stubbed out to handle it.  At the time I did not feel it was worth it to add an original party ID to the REDIRECTING function just for ISDN since that information was not going to change.  Since SS7 also has that information, I have added it.  (See https://reviewboard.asterisk.org/r/1829/ )
>     
>     For existing channel variables that REDIRECTING took over, what I did with ISDN is to make those channel variables status only and just use the values from REDIRECTING().  (You cannot have a source preference order if both sources are always present.)  PRIREDIRECTREASON is the principle example.
>     
>     The difference between the apparent duplicate redirecting information from SS7 could be a matter of the source supplying it.  This would be similar to the difference between Caller-ID and ANI.  Caller-ID is user supplied while ANI is network supplied.  Since the additional redirecting information is SS7 specific, it makes sense to use channel variables for that information.
>
> 
> KNK wrote:
>     The variables in question are not existing channel variables - they are added with this patch and meant to be set from the dialplan, so there is no need to keep them at all even as status only variables.
>     I will go ahead and replace them all with the REDIRECTING function, with the only exception of redirect_info_ind, which if provided will override the logic in place that generates it from the REDIRECTING data.

All done and tested - working as expected.

> On March 8, 2012, 4:22 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 2593-2610
> > <https://reviewboard.asterisk.org/r/1676/diff/2/?file=23474#file23474line2593>
> >
> >     This information should be obtained from the sig_ss7_indicate():AST_CONTROL_CONNECTED_LINE and ast_channel.connected.id.  Dialplan accesses these values using the CONNECTEDLINE function.  Continuing to add SS7 specific channel variables is not recommended.  See sig_pri for usage.
> 
> KNK wrote:
>     Moved connected line update from sig_ss7_answer to sig_ss7_indicate and replaced the variable with ast_channel_connected()

Connected line received from CPG, CON and ANM messages is also populated to ast_channel.connected.id

- KNK

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1676/#review5781
-----------------------------------------------------------

On April 23, 2012, 10:01 a.m., KNK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1676/
> -----------------------------------------------------------
> 
> (Updated April 23, 2012, 10:01 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> chan_dahdi / sig_ss7 part of changes
> 
> 
> This addresses bug SS7-27.
>     https://issues.asterisk.org/jira/browse/SS7-27
> 
> 
> Diffs
> -----
> 
>   trunk/channels/sig_ss7.h 363092 
>   trunk/channels/sig_ss7.c 363092 
>   trunk/channels/chan_dahdi.c 363092 
>   trunk/include/asterisk/channel.h 363092 
> 
> Diff: https://reviewboard.asterisk.org/r/1676/diff
> 
> 
> Testing
> -------
> 
> compiles, link setup, cli commands, bassic calls, connected line and redirection.
> 
> 
> Thanks,
> 
> KNK
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120423/d0c9aab3/attachment-0001.htm>