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

[Sean Bright]

> On 2009-06-24 18:38:16, rmudgett wrote:
> > 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.

So nothing would need to change in receive_reverse_charging_indication(), I just need to initialize q931_call.reversecharge to -1 in prepare_to_handle_q931_message, correct?

Also, I changed the enum values for reverse charge in libpri.h from a bitfield style definition (i.e. 1 << 0) to a constant integer.  I'm going on the assumption that if new values were added they would be mutually exclusive so there wouldn't be a need for flag style constants.  Is that a naive assumption?


> On 2009-06-24 18:38:16, rmudgett wrote:
> > /branches/1.4/q931.c, lines 2159-2162
> > <http://reviewboard.digium.com/r/292/diff/2/?file=5728#file5728line2159>
> >
> >     Change the test to reversecharge != PRI_REVERSECHARGE_NONE here.  This will allow potential other enum values to be sent.

Done


> On 2009-06-24 18:38:16, rmudgett wrote:
> > /branches/1.4/q931.c, lines 3218-3220
> > <http://reviewboard.digium.com/r/292/diff/2/?file=5728#file5728line3218>
> >
> >     Delete the test and just copy the value.  This will allow other potential enum values to be sent.

I had already thought to make that change but hadn't been back to the machine :)  Done


- Sean


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


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
> 
>