<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/2405/">https://reviewboard.asterisk.org/r/2405/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 21st, 2013, 11:46 a.m., <b>opticron</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2405/diff/2/?file=34895#file34895line339" style="color: black; font-weight: bold; text-decoration: underline;">/team/dlee/json_main/main/manager_channels.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void channel_snapshot_update(void *data, struct stasis_subscription *sub,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">335</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">manager_event</span><span class="p">(</span><span class="n">EVENT_FLAG_CALL</span><span class="p">,</span> <span class="s">"NewCallerid"</span><span class="p">,</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Are NewCallerid events also expected to be triggered simultaneously with other manager events from the same snapshot?</pre>
</blockquote>
<p>On March 21st, 2013, 11:48 a.m., <b>David Lee</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes. I'm not sure if it actually happens (OOM dropped events, maybe), but the code handles it in case it does.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Given the direction that the channel snapshot AMI event handling code is going and that more AMI events will be added to this soon, I'd prefer to handle all channel snapshot AMI events as if they could occur simultaneously (without adding a new variable for each one). As is, we're handling this two different ways depending on the AMI event to be generated and it will only make this callback more complicated as new AMI events are converted over to Stasis.</pre>
<br />
<p>- opticron</p>
<br />
<p>On March 21st, 2013, 10:52 a.m., David Lee wrote:</p>
<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, opticron and Matt Jordan.</div>
<div>By David Lee.</div>
<p style="color: grey;"><i>Updated March 21, 2013, 10:52 a.m.</i></p>
<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;">This patch builds upon the work in https://reviewboard.asterisk.org/r/2381/.
This moves the NewCallerid, HangupRequest and SoftHangupRequest events to be
based off of Stasis events.
HangupRequest and SoftHangupRequest are now ast_channel_blob Stasis messages,
with the cause code as an optional field in the blob.
NewCallerid now simply watches for changes in the callerid information in
channel snapshots, and creates the AMI event appropriately.
Since the original NewCallerid event honored the channelvars setting in
manager.conf, the channel variables configured there had to become a part of the
channel snapshot. These are now a part of every snapshot based event, making the
configuration description "every time a channel-oriented event is emitted" less
of a lie.
There a a few other changes wrapped up in here as well.
When ast_channel_topic() is given NULL for a channel, it returns the
ast_channel_topic_all() topic instead of NULL. This can clean up a lot of NULL
checking we're doing currently.
Additionally, the fields Cause and Cause-txt were removed from the base channel
information and put only on the Hangup events, since those fields are
meaningless outside of a Hangup event.
Oh, yeah, and I removed the pipe-delimiter processing of the channelvars field,
since that's been deprecated forever.</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;">Manually verified events looked compatible with original events.</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-21096">ASTERISK-21096</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>/team/dlee/json_main/CHANGES <span style="color: grey">(383514)</span></li>
<li>/team/dlee/json_main/include/asterisk/channel.h <span style="color: grey">(383514)</span></li>
<li>/team/dlee/json_main/main/channel.c <span style="color: grey">(383514)</span></li>
<li>/team/dlee/json_main/main/channel_internal_api.c <span style="color: grey">(383514)</span></li>
<li>/team/dlee/json_main/main/manager.c <span style="color: grey">(383514)</span></li>
<li>/team/dlee/json_main/main/manager_channels.c <span style="color: grey">(383514)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2405/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>