[asterisk-dev] [Code Review] 3633: Change in SETUP ACK handling (checking PI) in revision 413765 breaks working environments. This patch tries to make the change optional by adding a config option.

[rmudgett]

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



/branches/11/UPGRADE.txt
<https://reviewboard.asterisk.org/r/3633/#comment22354>

    Changing UPGRADE is not necessary.  This is fixing a regression.  Documenting the new option is only needed in chan_dahdi.conf.sample.



/branches/11/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/3633/#comment22356>

    Add
    .inband_on_setup_ack = 1,
    before
    .inband_on_proceeding
    to make the option default enabled.



/branches/11/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/3633/#comment22350>

    Make this unconditional.
    
    Also move to before the similar code for inband_on_proceeding.



/branches/11/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/3633/#comment22355>

    Move to before the similar code for inband_on_proceeding.



/branches/11/channels/sig_pri.h
<https://reviewboard.asterisk.org/r/3633/#comment22352>

    Change this to a bit field and move to before inband_on_proceeding.
    unsigned int inband_on_setup_ack:1;
    
    How about this description:
    TRUE to assume inband audio from a received SETUP ACK.
    



/branches/11/configs/chan_dahdi.conf.sample
<https://reviewboard.asterisk.org/r/3633/#comment22353>

    How about renaming to:
    inband_on_setup_ack
    
    This option works just like inband_on_proceeding but with SETUP ACK.  It also makes sense to document it before inband_on_proceeding since a SETUP ACK will always come before a PROCEEDING.
    
    Since this is fixing a regression, the default should be yes and when merged to trunk the default changed to no.
    
    Your description is talking about Asterisk internals which a user should not be expected to know about.
    
    ; Assume inband audio is present when a SETUP ACK message is received.
    ; Some ISDN switches do not give any indication that inband audio is
    ; present when they send a SETUP ACK message.  The receiver is assumed
    ; to know that if they send the message then there will be inband audio
    ; present for dialtone.
    ; Default yes in current release branches for backward compatibility.
    ;
    ;inband_on_setup_ack=yes



/branches/11/configs/chan_dahdi.conf.sample
<https://reviewboard.asterisk.org/r/3633/#comment22351>

    red blobs
    i.e. trailing whitespace


- rmudgett


On June 20, 2014, 6:24 a.m., Pavel Troller wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3633/
> -----------------------------------------------------------
> 
> (Updated June 20, 2014, 6:24 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23897
>     https://issues.asterisk.org/jira/browse/ASTERISK-23897
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The change introduced in revision 413765 limits opening the media channel and sending the PROGRESS control frame only to cases, when the Progress Indication with value "Inband Info now available" is present in the SETUP ACK message. However, there are exchanges (like Ericsson/Aastra MD110 BC13 or Avaya Definity (revision unknown)), which don't send this PI in SETUP ACK, but they send regular dial tone. Now the dial tone (and any subsequent audio information until some progress indication is received from the PBX) cannot be heard. My proposed solution is to introduce a new config option. I've called it "alwayssendprogress" and it's a binary no/yes option. In the "no" status, it keeps the new behaviour in place. In the "yes" status, it restores the old one.
> A better naming is possible, any proposals are welcome.
> The supplied patch was tested on current V11 SVN and it seems to be working as expected.
> 
> 
> Diffs
> -----
> 
>   /branches/11/configs/chan_dahdi.conf.sample 416805 
>   /branches/11/channels/sig_pri.c 416805 
>   /branches/11/channels/sig_pri.h 416805 
>   /branches/11/channels/chan_dahdi.c 416805 
>   /branches/11/UPGRADE.txt 416805 
> 
> Diff: https://reviewboard.asterisk.org/r/3633/diff/
> 
> 
> Testing
> -------
> 
> Regularly using the patch on three systems with no visible problems.
> 
> 
> File Attachments
> ----------------
> 
> Diff for Asterisk 1.8
>   https://reviewboard.asterisk.org/media/uploaded/files/2014/06/20/3c3b2124-78ce-41c8-b77f-e0bf4240b036__pri-1.8.diff
> 
> 
> Thanks,
> 
> Pavel Troller
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140620/0c76f4d1/attachment-0001.html>