[asterisk-dev] [Code Review] ASTERISK-18067 + ASTERISK-15479 - Peers with mailbox=a at b, c at d doesn't register c at d due to space, Asterisk notifies a peer about only one mailbox when receiving new voicemail

[jrose]

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

Review request for Asterisk Developers, Kevin Fleming, rmudgett, and mjordan.


Summary
-------

For the first issue, all I had to do was add the space character to the list of delimiter characters used by strsep.  Actually, this change is somewhat questionable if spaces are normally allowed in mailbox names.  I don't think they are, but I'm not certain.  If they are, please let me know so that I can remove that change.

Alternatively, we could simply search for the , and trim any whitespace until the next character.  Let me know if that's more ideal.

For the second issue, I removed use of event in the mwi notification building function because it would only ever check one mailbox and then skip the function to check cached events for the peer to tally the total number of old and new messages.  I was a little wary of that, but after talking with Richard I was a little more confident in doing that.


This addresses bugs ASTERISK-15479 and ASTERISK-18067.
    https://issues.asterisk.org/jira/browse/ASTERISK-15479
    https://issues.asterisk.org/jira/browse/ASTERISK-18067


Diffs
-----

  /branches/1.8/channels/chan_sip.c 331954 

Diff: https://reviewboard.asterisk.org/r/1365/diff


Testing
-------

Tested with sip.conf including spaces between mailboxes as well as commas and spaces.  Now space can be used to separate mailboxes.  Not sure whether that's a good thing or not, but it's easily fixable if it isn't.

Tested multiple mailboxes and single mailboxes to make sure subscribed mailboxes give mwi for the correct number of messages.  Specific scenarios include at the actual time of subscription and when receiving new voicemail.  All tests yielded appropriate voicemail counts in the mwi.


Thanks,

jrose

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110816/cd829fdf/attachment.htm>