----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3984/#review13267 -----------------------------------------------------------
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 <https://reviewboard.asterisk.org/r/3984/#comment23745> extended :-) branches/12/res/res_mwi_devstate.c <https://reviewboard.asterisk.org/r/3984/#comment23746> I don't think you need this include, since you're managing the message routing yourself. - Matt Jordan On Sept. 8, 2014, 1:21 p.m., Jason Parker wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3984/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2014, 1:21 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > Add a device state for MWI. This allows for a blinky light when a mailbox > has at least 1 new message. > > > Diffs > ----- > > branches/12/res/res_mwi_devstate.c PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/3984/diff/ > > > Testing > ------- > > 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/. > > > Thanks, > > Jason Parker > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
