[asterisk-dev] [Code Review] CCSS Proposal to add Asterisk DEVSTATEs to generic agents to use with hints
p_lindheimer
reviewboard at asterisk.org
Fri Feb 18 01:36:21 CST 2011
> On 2011-02-17 18:51:06, Paul Belanger wrote:
> > Few minor comments. Also, new features should be applied against trunk.
>
> p_lindheimer wrote:
> 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.
>
> Paul Belanger wrote:
> 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
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.
> On 2011-02-17 18:51:06, Paul Belanger wrote:
> > /branches/1.8/main/ccss.c, lines 4031-4032
> > <https://reviewboard.asterisk.org/r/1105/diff/3/?file=15562#file15562line4031>
> >
> > We'll need to document the new variables. UPGRADE.txt or wiki?
>
> p_lindheimer wrote:
> 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.
>
> Paul Belanger wrote:
> I'd suggest removing this logic and create a separate patch for issue 18763. We can triage that issue separately, or another reviewboard.
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.
- p_lindheimer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1105/#review3195
-----------------------------------------------------------
On 2011-02-17 20:46:51, p_lindheimer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1105/
> -----------------------------------------------------------
>
> (Updated 2011-02-17 20:46:51)
>
>
> Review request for Asterisk Developers.
>
>
> Summary
> -------
>
> 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.
>
>
> Diffs
> -----
>
> /branches/1.8/configs/ccss.conf.sample 308283
> /branches/1.8/main/ccss.c 308283
>
> Diff: https://reviewboard.asterisk.org/r/1105/diff
>
>
> Testing
> -------
>
> 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.
>
>
> Thanks,
>
> p_lindheimer
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110218/291476f5/attachment.htm>
More information about the asterisk-dev
mailing list