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

[rmudgett]

> On July 20, 2012, 9:34 p.m., rmudgett wrote:
> > trunk/channels/chan_dahdi.c, lines 19165-19167
> > <https://reviewboard.asterisk.org/r/1676/diff/6/?file=29255#file29255line19165>
> >
> >     You are not able to set/clear the flag explicitly.
> 
> KNK wrote:
>     I don't understand your comment, please elaborate.

You can only do:
ss7_default_echocontrol=yes
You cannot do:
ss7_default_echocontrol=no
because it cannot clear the flag.  It just does nothing.

This is true for the following config options:
ss7_explicitacm
ss7_autoacm
ss7_initialhwblo
ss7_use_echocontrol
ss7_default_echocontrol
there are more as well.


> On July 20, 2012, 9:34 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, line 657
> > <https://reviewboard.asterisk.org/r/1676/diff/6/?file=29257#file29257line657>
> >
> >     This should be testing bit the maintenance bit???
> 
> KNK wrote:
>     I don't see how this function can be reached with any blocking state as it always originates from sig_ss7_reset_cic() which clears local blocking and via ss7_start_rsc() which clears the remote blocking.
>     Also when we send RSC we expect to put the cic in service if it is not remotely blocked and upon receipt the remote host should clear its remoteblocking state, so it should be correct to have it always cleared.

ss7_do_rsc() is called by sig_ss7_hangup() and ss7_start_rsc() directly.
ss7_start_rsc() is called by sig_ss7_reset_cic().

isup_handle_unexpected() in libss7 can set the do_hangup to SS7_HANGUP_SEND_RSC which in sig_ss7_hangup() will then call ss7_do_rsc().


> On July 20, 2012, 9:34 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 978-980
> > <https://reviewboard.asterisk.org/r/1676/diff/6/?file=29257#file29257line978>
> >
> >     Why is this code here?  I had to do something like this for ISDN so the diversion-leg-3 message could be handled properly.  You don't seem to be doing anything with the information.
> 
> KNK wrote:
>     Yes, but i guess the information may be needed for debugging or somewhere else and it won't harm to have it here.

It should just be removed then.  The exten and calling presentation information is available elsewhere.


> On July 20, 2012, 9:34 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 2103-2105
> > <https://reviewboard.asterisk.org/r/1676/diff/6/?file=29257#file29257line2103>
> >
> >     Use ss7_find_cic_gripe() instead.
> 
> KNK wrote:
>     Done. I wonder how did i missed them.

I created the ss7_find_cic_gripe() function to eliminate the common gripe message and make the gripe format consistent.  I suspect you just ported the code from the v1.6 implementation without realizing that the ss7_find_cic_gripe() function was available.


> On July 20, 2012, 9:34 p.m., rmudgett wrote:
> > trunk/channels/sig_ss7.c, lines 2437-2443
> > <https://reviewboard.asterisk.org/r/1676/diff/6/?file=29257#file29257line2437>
> >
> >     Why not just trust the id.number.presentation value.
> 
> KNK wrote:
>     I am not sure if it is guaranteed to always be present. If so, it should be trusted.

I think it should be trusted because it is controllable by dialplan and the connected line interception routine.

Also id.number.presentation == 0 is a valid combination of the presentation and screening field values.  (SS7_PRESENTATION_ALLOWED, SS7_SCREENING_USER_PROVIDED_NOT_VERIFIED)


- rmudgett


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


On July 27, 2012, 2 p.m., KNK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1676/
> -----------------------------------------------------------
> 
> (Updated July 27, 2012, 2 p.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/chan_dahdi.c 370452 
>   trunk/channels/sig_ss7.h 370452 
>   trunk/channels/sig_ss7.c 370452 
> 
> 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/20120727/bbb6a8e7/attachment-0001.htm>