<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1939/">https://reviewboard.asterisk.org/r/1939/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and irroot.</div>
<div>By Matt Jordan.</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In prior versions of Asterisk, the lastmsgssent value was used to track whether or not a peer had received MWI notifications. Since chan_sip polled for the notifications itself, the value was rather important. When MWI notifications were changed to use the event notification framework, the value was no longer useful for anything other then reporting through the CLI or AMI events.
Hence, in Asterisk 10 and trunk, the value was completely removed. Unfortunately, in Asterisk 1.8, the value was not removed; instead, it is set to a value of -1 and never updated. Since the lower 16 bits are used for old messages and the upper 16 bits are used for new messages, this results in the following being displayed for 'sip show peer foo':
LastMsgsSent : 32767/65535
Normally, I'd suggest that we remove the field from Asterisk 1.8 and call it a day. However, since this field was supplied to users via AMI and the CLI, doing so breaks backwards compatibility.
This patch is a modification of a patch originally supplied by irroot on ASTERIS-17866. It re-implements updating of lastmsgssent when an MWI notification is sent to a SIP peer. Note that the original patch had to be modified slightly due to changes in sip_send_mwi_to_peer (and that up to three threads can be involved in accessing lastmsgssent means that, at the very least, the peer really should be locked when we update it).
If we decide that this isn't worth it, we should instead remove the field from Asterisk 1.8, as reporting an erroneous value isn't terribly useful.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested with two SIP realtime peers. One peer without voicemail continued to display the 32767/65535 value - which is expected, as that value indicates that we haven't sent any message notifications. Another peer, with voicemail, correctly showed the new/old message counts post registration, and displayed the counts correctly after MWI notifications were sent when new voicemails were left or listened to.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-17866">ASTERISK-17866</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(367134)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1939/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>