[asterisk-dev] [Code Review] ETSI Message Waiting Indication (MWI) (Asterisk component)

[Russell Bryant]

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



/team/group/CCSS/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/599/#comment3865>

    Why does this need a new option instead of the existing "mailbox" option?



/team/group/CCSS/channels/sig_pri.h
<https://reviewboard.asterisk.org/r/599/#comment3866>

    Why 8?  Why any limit at all?



/team/group/CCSS/channels/sig_pri.h
<https://reviewboard.asterisk.org/r/599/#comment3867>

    The buffer size here is a bit of a magic number.  If you're going to use AST_MAX_EXTENSION, it seems that multiplying that by the number of mailboxes possible would be more appropriate.  That's a lot of wasted space, though.  Perhaps this object needs to be updated to use the stringfields API?



/team/group/CCSS/channels/sig_pri.h
<https://reviewboard.asterisk.org/r/599/#comment3868>

    docs for the new sig_pri API call?



/team/group/CCSS/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/599/#comment3869>

    Use ast_debug()



/team/group/CCSS/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/599/#comment3870>

    I'd rather see ARRAY_LEN() on pri->mbox than the #define used



/team/group/CCSS/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/599/#comment3871>

    Could this be reworked to use strsep()?  Asterisk code pretty much uses strsep() everywhere instead of strtok_r()


- Russell


On 2010-03-31 11:41:00, rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/599/
> -----------------------------------------------------------
> 
> (Updated 2010-03-31 11:41:00)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Add the ability to report waiting messages to ISDN endpoints (phones).
> 
> Relevant specification: EN 300 650 and EN 300 745
> 
> This is based off of the team/group/CCSS branch to get the
> sig_pri_load()/sig_pri_unload() improvements which turned out to not be
> necessary.
> 
> The libpri portion of this review is locatged in review:
> https://reviewboard.asterisk.org/r/600/
> 
> 
> Diffs
> -----
> 
>   /team/group/CCSS/CHANGES 255495 
>   /team/group/CCSS/channels/chan_dahdi.c 255495 
>   /team/group/CCSS/channels/sig_pri.h 255495 
>   /team/group/CCSS/channels/sig_pri.c 255495 
>   /team/group/CCSS/configs/chan_dahdi.conf.sample 255495 
>   /team/group/CCSS/configure.ac 255495 
> 
> Diff: https://reviewboard.asterisk.org/r/599/diff
> 
> 
> Testing
> -------
> 
> 1) Setup app_voicemail
> 2) Left messages in various voice mail contexts
> 3) Retrieved messages
> 
> Verified that the MWIIndication message reported the expected number of waiting messages.
> Also verified that the MWIIndication message is reported when Asterisk is started.
> 
> 
> Thanks,
> 
> rmudgett
> 
>