Review Board 1.7.16


Module to add devstate for MWI.

Review Request #3984 - Created Sept. 8, 2014 and updated

Jason Parker
branches/12/
Reviewers
asterisk-dev
Asterisk
Add a device state for MWI.  This allows for a blinky light when a mailbox has at least 1 new message.
Leaving a voicemail then deleting that voicemail causes the hint to go to IN_USE then to NOT_INUSE.  Polled mailboxes do not appear to update, but I can't tell whether that's a failing of pollmailboxes or not.

Patch also applies to and functions on branches/13/.
Total:
2
Open:
2
Resolved:
0
Dropped:
0
Status:
From:
Description From Last Updated Status
extended :-) Matt Jordan Sept. 9, 2014, 9:39 a.m. Open
I don't think you need this include, since you're managing the message routing yourself. Matt Jordan Sept. 9, 2014, 9:39 a.m. Open
Review request changed
Updated (Sept. 8, 2014, 1:21 p.m.)
  • Leaving a voicemail then deleting that voicemail causes the hint to go to IN_USE then to NOT_INUSE.  Polled mailboxes do not appear to update, but I can't tell whether that's a failing of pollmailboxes or not.

    Leaving a voicemail then deleting that voicemail causes the hint to go to IN_USE then to NOT_INUSE.  Polled mailboxes do not appear to update, but I can't tell whether that's a failing of pollmailboxes or not.
    
    Patch also applies to and functions on branches/13/.
Update testing.
Posted (Sept. 9, 2014, 9:39 a.m.)
This looks really good. Nice job!

It'd be good to get a test for this - not because the module itself has any problems, but to prevent it getting broken from a bad change to the APIs that it depends on (Stasis, device state, mailbox core, etc.) Having a test puts the burden of fixing inadvertent problems on the initial committer, as opposed to discovering it when FreePBX breaks and having the issues get pushed to you - only to discover that someone else broke the module.

You could write a test for this very easily using the external MWI functionality. A collection of tests that are based on that functionality can be seen here:

https://bamboo.asterisk.org/bamboo/browse/AST-ATSEM-C632TE-210/test

Essentially, you could use a small Python script to update the mailbox count on a mailbox, then check that the appropriate device state changed. If you need a hand writing the test, let me know and I can pitch in on it.
branches/12/res/res_mwi_devstate.c (Diff revision 1)
 
 
extended :-)
branches/12/res/res_mwi_devstate.c (Diff revision 1)
 
 
I don't think you need this include, since you're managing the message routing yourself.
Ship it!
Posted (Sept. 12, 2014, 2:01 p.m.)
Aside from what Matt had to say, this looks good by me.

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.