[asterisk-dev] [Code Review] CCSS Proposal to add Asterisk DEVSTATEs to generic agents to use with hints

[rmudgett]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1105/#review3273
-----------------------------------------------------------


Mostly nit-picking.  I have committed your related bug fix patch with modifications for issue #18763.


/trunk/configs/ccss.conf.sample
<https://reviewboard.asterisk.org/r/1105/#comment6765>

    These state -> These states



/trunk/configs/ccss.conf.sample
<https://reviewboard.asterisk.org/r/1105/#comment6766>

    funciton -> function



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6775>

    internal -> internal state



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6768>

    statemachine -> state machine



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6774>

    CC_FAIL -> CC_FAILED



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6769>

    and -> any



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6767>

    CCSS has its own dynamic logging channel defined.
    
    You should use ast_log_dynamic_level(cc_logger_level, ...) instead of ast_debug().
    
    The output is accessed by adding the CC channel to the various logging lists in logger.conf.



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6773>

    Use tabs to indent.



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6770>

    Add const qualifier to cc_setting.



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6771>

    This return is not needed.



/trunk/main/ccss.c
<https://reviewboard.asterisk.org/r/1105/#comment6772>

    Return not needed


- rmudgett


On 2011-02-20 21:18:19, p_lindheimer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1105/
> -----------------------------------------------------------
> 
> (Updated 2011-02-20 21:18:19)
> 
> 
> 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
> -----
> 
>   /trunk/CREDITS 308370 
>   /trunk/configs/ccss.conf.sample 308370 
>   /trunk/main/ccss.c 308370 
> 
> 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/20110402/a899df45/attachment-0001.htm>