<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/3771/">https://reviewboard.asterisk.org/r/3771/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<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-24038">ASTERISK-24038</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</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 Asterisk, it is possible for a device to have a status of ONHOLD. This is not typically an easy thing to determine, as a channel being on hold is not a direct channel state. Typically, this has to be calculated outside of the core independently in channel drivers, notably, chan_sip and chan_pjsip. Both of these channel drivers already have to calculate device state in a fashion more complex than the core can handle, as they aggregate all state of all channels associated with a peer/endpoint; they also independently track whether or not one of those channels is currently on hold and mark the device state appropriately.
In 12+, we now have the ability to report a AST_DEVICE_ONHOLD state for all channels that defer their device state to the core. This is due to channel hold state actually now being tracked on the channel itself. If a channel driver defers its device state to the core (which many, such as DAHDI, IAX2, and others do in most situations), the device state core already goes out to get a channel associated with the device. As such, it can now also factor the channel hold state in its calculation.
This patch adds this logic to the device state core. It also uses an existing mapping between device state and channel state to handle more channel states. chan_pjsip has been updated slightly as well to make use of this (as it was, for some reason, reporting a channel state of BUSY as a device state of INUSE, which feels slightly wrong).</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;">Some manual testing was done. Additionally, the pjsip hold test was modified to report back the device state using the DEVICE_STATE function which verified the correct hold state coming back from that channel driver.
New tests should be added to check the core logic; this review will be updated when those tests are added.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/main/devicestate.c <span style="color: grey">(418565)</span></li>
<li>/trunk/channels/chan_pjsip.c <span style="color: grey">(418565)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3771/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>