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

[rmudgett at digium.com]

> 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.
> 
> Sean Bright wrote:
>     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?

Yes, just initialize the reversecharge to -1.  There does not seem to be anything else to do for that in libpri.  Of course, there needs to be a corresponding change in chan_dahdi to handle the new value meanings.

I had thought of commenting on the bitfield definition you had used, but thought it could also be interpreted as shifting an enumeration value up to the field location.  If any new values are added I would still just consider it a three bit enumeration field.  There are four spare bits available in that octet to add more features if needed.


- rmudgett


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


On 2009-06-24 22:13:41, Sean Bright wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/292/
> -----------------------------------------------------------
> 
> (Updated 2009-06-24 22:13:41)
> 
> 
> 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
> 
>