[asterisk-dev] [Code Review] device state for queue memeber pause

[Russell Bryant]

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


My comments on the code are very minor.  The only thing holding me back from a "Ship It" is that this isn't documented anywhere.  Unfortunately, I'm not even sure where to document it.  :-(  It needs to at least be in CHANGES, though.  In the absence of any better ideas, a section about Queue related device state providers could be added to queues.conf.sample.


http://svn.asterisk.org/svn/asterisk/trunk/apps/app_queue.c
<https://reviewboard.asterisk.org/r/733/#comment5272>

    You can break out of this loop once paused is set to non-zero.



http://svn.asterisk.org/svn/asterisk/trunk/apps/app_queue.c
<https://reviewboard.asterisk.org/r/733/#comment5271>

    Use ast_debug() here.


- Russell


On 2010-06-22 13:43:07, tim_ringenbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/733/
> -----------------------------------------------------------
> 
> (Updated 2010-06-22 13:43:07)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds a device state provider named "qpaused" in the app_queue.c module.
> It's used with a hint in style of "exten => 8888,hint,qpaused:SIP/test1", and the hint returns in use if the interface exists and is paused in any queue, otherwise it returns not in use.
> 
> It also patches devicestate.c to allow a "/" character. I think it's a bug that it isn't allowed to begin with.
> 
> It uses "AST_DEVICE_UNKNOWN" in the ast_devstate_changed calls partly because the patch was originally against 1.4, but also because app_queue really doesn't know the answer without running the callback itself, and it can't easily do that due to locking issues.
> 
> I have a really similar patch for "qloggedin" instead of paused that I still need to port to trunk. I'm going to wait and see if this patch gets accepted or not first since they're similar, so if the community wants this patch reworked or rejects it they'll probably want them both reworked or reject them both.
> 
> The use case is to have a speeddial queue pause/unpause button with a busy lamp field that lights up when paused, isn't lit up when not paused. I don't want the light to blink when they're not logged in to any queue, which is why it returns not in use instead of invalid in that case.
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/trunk/apps/app_queue.c 271972 
>   http://svn.asterisk.org/svn/asterisk/trunk/main/devicestate.c 271972 
> 
> Diff: https://reviewboard.asterisk.org/r/733/diff
> 
> 
> Testing
> -------
> 
> This version of the patch has minimal testing (compiling, core show hints, queue pause, core show hints). The 1.4 version is in production at at least one site actually using the functionality, and several others that don't use it but have the patch anyway.
> 
> 
> Thanks,
> 
> tim_ringenbach
> 
>