[asterisk-dev] [Code Review] 2694: Clean up queue operations (get rid of BUGBUGs)

[Mark Michelson]

> On Aug. 19, 2013, 6:12 p.m., opticron wrote:
> > /trunk/apps/app_queue.c, lines 5143-5145
> > <https://reviewboard.asterisk.org/r/2694/diff/2/?file=43587#file43587line5143>
> >
> >     The peer and caller snapshots should be handled if NULL in this function as queue_publish_multi_channel_snapshot_blob does not check for NULL snapshots and callers of send_agent_complete could pass NULL snapshots into this function.

It should not be possible to pass NULL snapshots into this function. If we do pass NULL snapshots into this function, then something very bad has happened. I'll add assertions to make sure that the passed-in snapshots are not NULL.


> On Aug. 19, 2013, 6:12 p.m., opticron wrote:
> > /trunk/apps/app_queue.c, line 5281
> > <https://reviewboard.asterisk.org/r/2694/diff/2/?file=43587#file43587line5281>
> >
> >     Since the channel and bridge unsubscriptions are handled elsewhere, this should assert that both are NULL or handle them properly if they're not.


> On Aug. 19, 2013, 6:12 p.m., opticron wrote:
> > /trunk/apps/app_queue.c, lines 5363-5369
> > <https://reviewboard.asterisk.org/r/2694/diff/2/?file=43587#file43587line5363>
> >
> >     These should be ast_str_set() instead of ast_str_append().


- Mark


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


On Aug. 1, 2013, 8:50 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2694/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 8:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21517 and ASTERISK-21943
>     https://issues.asterisk.org/jira/browse/ASTERISK-21517
>     https://issues.asterisk.org/jira/browse/ASTERISK-21943
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Large-scale bridge refactoring left app_queue in a state with lots of commented-out code and many BUGBUG comments.
> 
> This changeset aims to improve app_queue to get back to proper operation. The following changes were made:
> 
> * Reporting of transfers and call completion is done by creating stasis subscriptions and listening for specific events in order to determine when the call is finished (either via a transfer or hangup).
> * Dial end messages have been added where they were previously missing.
> * Queue stats are properly being updated again once calls have finished.
> * AgentComplete stasis messages and AMI events are now occurring again.
> * Mixmonitor starting has been factored into its own function and uses the Mixmonitor API now instead of using ast_pbx_run()
> 
> In addition to the changes in app_queue, there are several supplementary changes as well:
> 
> * Queue logging now differentiates between attended and blind transfers. A note about this is in the CHANGES file.
> * Local channel optimization events now report more information. This includes which of the two local channels involved is the destination of the optimization, the channel that is replacing the destination local channel, and an identifier so that begin and end events can be matched to each other. The end events are now sent whether the optimization was successful or not and includes an indicator of whether the optimization was successful.
> * Changes were made to features and bridging_basic so that additional flags may be set on a bridge. This is necessary because the queue requires that its bridge only allows move-swap local channel optimizations into the bridge.
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 396027 
>   /trunk/apps/app_queue.c 396027 
>   /trunk/include/asterisk/app.h 396027 
>   /trunk/include/asterisk/bridge_basic.h 396027 
>   /trunk/include/asterisk/core_unreal.h 396027 
>   /trunk/include/asterisk/features.h 396027 
>   /trunk/main/app.c 396027 
>   /trunk/main/bridge.c 396027 
>   /trunk/main/bridge_basic.c 396027 
>   /trunk/main/core_local.c 396027 
>   /trunk/main/features.c 396027 
> 
> Diff: https://reviewboard.asterisk.org/r/2694/diff/
> 
> 
> Testing
> -------
> 
> Loads.
> 
> The biggest thing that needed to be checked was that hangups and transfers were detected properly. I tested various local channel optimization scenarios to ensure that they worked as expected. I also ensured that the mixmonitor changes worked as expected.
> 
> 
> Thanks,
> 
> Mark Michelson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130819/bb52fd69/attachment-0001.htm>