[asterisk-dev] [Code Review]: Stasis application WebSocket support

[David Lee]

> On March 21, 2013, 9:18 a.m., opticron wrote:
> > /trunk/apps/app_stasis.c, lines 88-99
> > <https://reviewboard.asterisk.org/r/2361/diff/5/?file=34763#file34763line88>
> >
> >     These ref-bumping accessors are unnecessary since the containers are only created and have their final unref on module load and unload.

Not necessary, but they are reffed and unreffed throughout the file. I found the wrapper preferable to the explicit ao2_ref.


> On March 21, 2013, 9:18 a.m., opticron wrote:
> > /trunk/apps/app_stasis.c, line 101
> > <https://reviewboard.asterisk.org/r/2361/diff/5/?file=34763#file34763line101>
> >
> >     There are multiple different local symbols in this review named "stasis_app" which could be confusing and they should not match stasis_* which denotes global symbols.

Agreed.


> On March 21, 2013, 9:18 a.m., opticron wrote:
> > /trunk/apps/app_stasis.c, line 315
> > <https://reviewboard.asterisk.org/r/2361/diff/5/?file=34763#file34763line315>
> >
> >     msg needs a null check.

fixed.


> On March 21, 2013, 9:18 a.m., opticron wrote:
> > /trunk/apps/app_stasis.c, line 328
> > <https://reviewboard.asterisk.org/r/2361/diff/5/?file=34763#file34763line328>
> >
> >     Idem.

fixed


> On March 21, 2013, 9:18 a.m., opticron wrote:
> > /trunk/res/res_stasis_websocket.c, line 51
> > <https://reviewboard.asterisk.org/r/2361/diff/5/?file=34772#file34772line51>
> >
> >     This is the other stasis_app.

silly me.


> On March 21, 2013, 9:18 a.m., opticron wrote:
> > /trunk/res/res_stasis_websocket.c, lines 257-259
> > <https://reviewboard.asterisk.org/r/2361/diff/5/?file=34772#file34772line257>
> >
> >     This error message is also applicable in several other places, but is unnecessary.

agreed.


- David


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


On March 18, 2013, 12:47 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2361/
> -----------------------------------------------------------
> 
> (Updated March 18, 2013, 12:47 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch is based on the stasis-core message bus, in
> https://reviewboard.asterisk.org/r/2339/
> 
> This is the API that binds the Stasis dialplan application to external
> Stasis applications. It also adds the beginnings of WebSocket
> application support.
> 
> This module registers a dialplan function named Stasis, which is used
> to put a channel into the named Stasis app. As a channel enters and
> leaves the Stasis diaplan applcation, the Stasis app receives a
> 'stasis-start' and 'stasis-end' events.
> 
> Stasis apps register themselves using the stasis_app_register and
> stasis_app_unregister functions. Messages are sent to an appliction
> using stasis_app_send.
> 
> Finally, Stasis apps control channels through the use of the
> stasis_app_control object, and the family of stasis_app_control_*
> functions.
> 
> The biggest unknown I have about this patch is how I'm exporting
> symbols from app_stasis.so. I believe I'm doing it correctly, and they
> should not need to use optional API. If anyone knows if it will be a
> problem on BSD, Solaris, etc., please let me know.
> 
> Other changes along for the ride are:
>  * An ast_frame_dtor function that's RAII_VAR safe
>  * Some common JSON encoders for name/number, timeval, and
>    context/extension/priority
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/Makefile 383287 
>   /trunk/apps/app_stasis.c PRE-CREATION 
>   /trunk/apps/app_stasis.exports.in PRE-CREATION 
>   /trunk/apps/stasis_json.c PRE-CREATION 
>   /trunk/include/asterisk/app_stasis.h PRE-CREATION 
>   /trunk/include/asterisk/frame.h 383287 
>   /trunk/include/asterisk/json.h 383287 
>   /trunk/include/asterisk/localtime.h 383287 
>   /trunk/main/frame.c 383287 
>   /trunk/res/res_json.c 383287 
>   /trunk/res/res_stasis_websocket.c PRE-CREATION 
>   /trunk/tests/test_abstract_jb.c 383287 
>   /trunk/tests/test_app_stasis.c PRE-CREATION 
>   /trunk/tests/test_json.c 383287 
> 
> Diff: https://reviewboard.asterisk.org/r/2361/diff
> 
> 
> Testing
> -------
> 
> Unit tests; ran a few channels through the applications
> 
> 
> Thanks,
> 
> David
> 
>

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