[asterisk-dev] [Code Review] Add support for sending Reverse Charging Indication IE on ISDN PRI

[rmudgett at digium.com]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/292/#review880
-----------------------------------------------------------

Ship it!


Other than the items noted below, the change looks good.

Slightly off topic for this review:  You may want to change the receiving of the reverse charging indication to allow for the possibility of other enum values.  The PRI_EVENT_RING reversecharge value could then contain: -1 (Normal charging), 0-7.  Then chan_dahdi could set PRIREVERSECHARGE to the received value.  The dialplan could then check if PRIREVERSECHARGE is not negative for a reverse charged call.


/branches/1.4/q931.c
<http://reviewboard.digium.com/r/292/#comment2130>

    Change the test to reversecharge != PRI_REVERSECHARGE_NONE here.  This will allow potential other enum values to be sent.



/branches/1.4/q931.c
<http://reviewboard.digium.com/r/292/#comment2131>

    Delete the test and just copy the value.  This will allow other potential enum values to be sent.


- rmudgett


On 2009-06-24 17:02:51, Sean Bright wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/292/
> -----------------------------------------------------------
> 
> (Updated 2009-06-24 17:02:51)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> As requested by Jared, this adds the ability for LibPRI to send a Reverse Charging Indication IE within a Q931 SETUP message.
> 
> I will admit to not being a LibPRI expert, but this change seems safe enough.
> 
> 
> This addresses bug 13760.
>     http://issues.asterisk.org/view.php?id=13760
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/libpri.h 885 
>   /branches/1.4/pri.c 884 
>   /branches/1.4/pri_internal.h 885 
>   /branches/1.4/q931.c 885 
> 
> Diff: http://reviewboard.digium.com/r/292/diff
> 
> 
> Testing
> -------
> 
> Compilation testing only.
> 
> 
> Thanks,
> 
> Sean
> 
>