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

[paul.belanger at polybeacon.com]

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


Peer review for CODING-GUIDELINES


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

    braces around if statements



http://svn.asterisk.org/svn/asterisk/trunk/main/devicestate.c
<https://reviewboard.asterisk.org/r/733/#comment4749>

    same comment


- pabelanger


On 2010-06-17 17:35:28, tim_ringenbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/733/
> -----------------------------------------------------------
> 
> (Updated 2010-06-17 17:35:28)
> 
> 
> 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 271231 
>   http://svn.asterisk.org/svn/asterisk/trunk/main/devicestate.c 271231 
> 
> 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
> 
>