<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/1105/">https://reviewboard.asterisk.org/r/1105/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 17th, 2011, 6:51 p.m., <b>Paul Belanger</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;">Few minor comments. Also, new features should be applied against trunk.</pre>
</blockquote>
<p>On February 17th, 2011, 8:23 p.m., <b>p_lindheimer</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;">sorry, didn't see this comment. The patch does apply against trunk.
I honestly believe that this feature is needed to properly use a generic agent implementation of CCSS due to the asynchronous nature of calls like CallCompletionRequest() which can't conclusively determine if the request was successful and thus dialplan can't be written to fully confirm. WIth this feature, this can now be accomplished. I didn't file it as a bug because it's obviously well more than a bug, but I did it for now against 1.8 because I still feel that there should be some serious consideration into 'finishing what was started' wrt to CCSS and putting it into 1.8. As can be seen from the changes, they are minimal and there is no legacy installed base since CCSS is brand new in 1.8, so no potential impact there. As it stands, right now the FreePBX CCSS module is being written to expect this functionality. Once final decisions are made we'll deal with the outcome.</pre>
</blockquote>
<p>On February 17th, 2011, 10:46 p.m., <b>Paul Belanger</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;">We have a policy in place that does not permit[1] new features to be added to release branches. So, once the code is merged into trunk, it will be tagged for the next release, 1.10.
[1] https://wiki.asterisk.org/wiki/display/AST/Issue+Tracker+Workflow#IssueTrackerWorkflow-ReleaseBranchCommitPolicy</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks Paul. I am aware of the policy and I have discussed with Russell who has indicated it almost certainly would not make it. However it's still worth getting feedback from other devs. The specific policy is"The only time the release branches will be changed is to fix a bug. New features will never be included in the release branch unless a special exception is made by the release branch maintainers." An example where this policy had an exception was to add device state information to queue members around 1.4.25 to provide a solution that was very difficult to work around without the new feature, and that was VERY MUCH a new feature. This also gets around problems that are otherwise not really solvable without it or something similar. The big difference between the two is that this has not been around long enough to hear about the problems of not having this sort of state information, and we are VERY early in the 1.8 life (which is not a bad time to rectify such things). (And I do recall being told at Astricon 09 when we went back to the current release plans that some level of compromise was going to be made to handle situations like this and come up with a median between how things were pre 1.6 and now.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 17th, 2011, 6:51 p.m., <b>Paul Belanger</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/1105/diff/3/?file=15562#file15562line4031" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/main/ccss.c</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 int ccreq_exec(struct ast_channel *chan, const char *data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3927</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="o">-</span><span class="mi">1</span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4031</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">pbx_builtin_setvar_helper</span><span class="p">(</span><span class="n">chan</span><span class="p">,</span> <span class="s">"CC_REQUEST_RESULT"</span><span class="p">,</span> <span class="s">"FAIL"</span><span class="p">);</span></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">4032</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">pbx_builtin_setvar_helper</span><span class="p">(</span><span class="n">chan</span><span class="p">,</span> <span class="s">"CC_REQUEST_REASON"</span><span class="p">,</span> <span class="s">"NO_CORE_INSTANCE"</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;">We'll need to document the new variables. UPGRADE.txt or wiki?</pre>
</blockquote>
<p>On February 17th, 2011, 8:09 p.m., <b>p_lindheimer</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;">This is actually part of: https://issues.asterisk.org/view.php?id=18763 so won't be in here, just have not backed it out of my working directory since it fixes the bug.</pre>
</blockquote>
<p>On February 17th, 2011, 10:46 p.m., <b>Paul Belanger</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;">I'd suggest removing this logic and create a separate patch for issue 18763. We can triage that issue separately, or another reviewboard.</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;">There is a patch attached to issue 18763, that's what is combined here. In any event, next role of the diff I'll drop it into trunk and back out that patch so that we can keep this separate.</pre>
<br />
<p>- p_lindheimer</p>
<br />
<p>On February 17th, 2011, 8:46 p.m., p_lindheimer wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/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.</div>
<div>By p_lindheimer.</div>
<p style="color: grey;"><i>Updated 2011-02-17 20:46:51</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;">CCSS Proposal to add Asterisk DEVSTATE to generic agents to use in hints
originally submitted to: https://issues.asterisk.org/view.php?id=18788
Proposal to Add Asterisk Device State information and callbacks to the
Call Completion Supplemental Services for generic agents.
There are currently not many devices that have native support for CCSS and
even as they come available there may be other reasons why one may choose not
to take advantage of the native abilities and stick with the generic
implementation which is quite capable and could be greatly enhanced by adding
device state capabilities that a phone could subscribe to with a BLF key in
conjuction with Asterisk hints.
The advantages of this would allow a single button that could be programmed to
trigger a CCSS related feature code that can be used to both Request and
Cancel CCSS requests as well as display the current state of a CCSS Request.
By example, I may have a single button that when no lit, there is no active
CCSS request. When I press that button, my dialplan can query the state
DEVICE_STATE() associated with that Caller to determine whether they should be
calling CallCompletionRequest() or CallCompletionCancel(). If there is
currently a pending request, then the dialplan would Cancel() it. This also has
the advantage of showing the true state of a Request() which is an
asynchronous call and even when Request() thinks it was successful, the actual
request could ultimately fail. Once lit, further feedback can now be provided
to the Caller about the current state of their Request() since it will be
updated by the CCSS State Machine as appropriate.
The DEVICE_STATE mappings should be configurable since the BLF being
used on a give phone type may vary. The idea is to allow some level of
customization as to the phone's behavior.
As an example, I may want my BLF key to go solid once I have requested a
callback. I may then want my led to blink (typically ringing) when either the
callback is in process which is a visual indication that the incoming call is
the desired callback, or I may want it to blink when the callee is ready buy I
am busy, giving me a visual indication that the target is available as I may
want to get off the line so that the callback can be successful.
The proposal is to send device state information back via the
ast_devstate_prov_add callback for any device (or at least generic device)
as they traverse through the state machine. We simply provide a map
between CC_STATE values and corresponding AST_DEVICE state values.
We could then generate hints against these States similar to what is possible
today with Custom Devstates or MeetMe states. By example, I may have an extension
3000 that is currently associated with device SIP/3000. I could then create a
feature code for that extension that may look something like:
exten => *823000,hint,ccss:sip/3000
One would then subscribe a blf button to *823000 which would point to the dialplan
that handled my CCSS Requests/Cancels using the available DEVICE_STATE()
information about ccss:sip/3000 to make the decsions what to do.
NOTICE: This patch happens to have a proposed bug fix patch in it as well from ticket:
https://issues.asterisk.org/view.php?id=18763
I just noticed that, not related to the proposed change, but does fix the fact that
CallCompletionRequest() and CallCompletionCancel() fail with non-zero codes in some
circumstances.</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;">Testing that has been done so far:
Created 2 phones:
SIP/3000 and SIP/3001
Created hints and some simple dialplan for testing:
exten => *823000,hint,ccss:SIP/3000
exten => *823000,1,Noop(STATE OF HINT IS ${DEVICE_STATE(ccss:SIP/3000)}})
exten => *823001,hint,ccss:SIP/3001
exten => *823001,1,Noop(STATE OF HINT IS ${DEVICE_STATE(ccss:SIP/3001)}})
Ran through scenarios with CCSS checking the values with 'core show hints' as different states were introduced including the main ones of:
CC_ACTIVE, CC_CALLEE_READY, CC_CALLEE_BUSY, CC_RECALLING
Tested changing the defaults with a single entry in ccss.conf of:
cc_caller_busy_devstate = RINGING
and verified that the CC_CALLEE_BUSY changed from the RINGINUSE to the RINGING state when tested.
Verified general operation of CCSS was not changed from these new addtions.
</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>/branches/1.8/configs/ccss.conf.sample <span style="color: grey">(308283)</span></li>
<li>/branches/1.8/main/ccss.c <span style="color: grey">(308283)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1105/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>