[asterisk-dev] [Code Review]: Move Newexten and UserEvent AMI events to Stasis; adding JSON messages to stasis-core

[David Lee]

> On March 18, 2013, 8:43 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, line 178
> > <https://reviewboard.asterisk.org/r/2381/diff/4/?file=34691#file34691line178>
> >
> >     The syntax documentation this patch adds need to include the xi:include tag for the Newchannel event since it holds the documentation for the standard set of channel event fields.

added.


> On March 18, 2013, 8:43 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, lines 299-301
> > <https://reviewboard.asterisk.org/r/2381/diff/4/?file=34691#file34691line299>
> >
> >     This is missing syntax information entirely.

added.


> On March 18, 2013, 8:43 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, line 302
> > <https://reviewboard.asterisk.org/r/2381/diff/4/?file=34691#file34691line302>
> >
> >     Extra 'o'.

removed.


> On March 18, 2013, 8:43 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, lines 326-334
> > <https://reviewboard.asterisk.org/r/2381/diff/4/?file=34691#file34691line326>
> >
> >     Idem.

added.


> On March 18, 2013, 8:43 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, line 237
> > <https://reviewboard.asterisk.org/r/2381/diff/4/?file=34691#file34691line237>
> >
> >     Instead of setting the precedent of using new variables to confer that more information be added to the AMI event, I suggest using an additional ast_str.  This additional ast_str can then be added to the channel state blob before being sent out.  This would also allow the Newexten event to use the same mechanism as the other channel state messages.

I'm with you on using ast_str, but I'm less sure about using it for Newexten. There's a bit of extra logic, and you may have a single snapshot fire both a Newstate and Newexten at the same time.


- David


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


On March 15, 2013, 6:39 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2381/
> -----------------------------------------------------------
> 
> (Updated March 15, 2013, 6:39 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch started out simply as fixing the bouncing tests introduced
> in r382685, but required some other changes to give it a decent
> implementation.
> 
> To fix the bouncing tests, the UserEvent and Newexten AMI events
> needed to be refactored to dispatch via Stasis. Dispatching directly
> to AMI resulted in those events sometimes getting ahead of the
> associated Newchannel events, which would understandably confuse anyone.
> 
> I found that instead of creating a zillion different message types and
> structures associated with them, it would be preferable to define a
> message type that has a channel snapshot and a blob of structured data
> with a small bit of additional information. The JSON object model
> provides a very nice way of representing structured data, so I went
> with that.
> 
>  * Move JSON support from res_json.c to main/json.c
>    * Made libjansson-dev a required dependency
>  * Added an ast_channel_blob message type, which has a channel
>    snapshot and JSON blob of data.
>  * Changed UserEvent and Newexten events so that they are dispatched
>    via ast_channel_blob messages on the channel's topic.
>  * Got rid of the ast_channel_varset message; used ast_channel_blob
>    instead.
>  * Extracted the manager functions converting Stasis channel events to
>    AMI events into manager_channel.c.
> 
> 
> This addresses bug ASTERISK-21096.
>     https://issues.asterisk.org/jira/browse/ASTERISK-21096
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 383261 
>   /trunk/apps/app_userevent.c 383261 
>   /trunk/configure UNKNOWN 
>   /trunk/configure.ac 383261 
>   /trunk/include/asterisk/autoconfig.h.in 383261 
>   /trunk/include/asterisk/channel.h 383261 
>   /trunk/include/asterisk/manager.h 383261 
>   /trunk/main/channel.c 383261 
>   /trunk/main/json.c PRE-CREATION 
>   /trunk/main/manager.c 383261 
>   /trunk/main/manager_channels.c PRE-CREATION 
>   /trunk/main/pbx.c 383261 
>   /trunk/pbx/pbx_realtime.c 383261 
>   /trunk/res/res_json.c 383261 
>   /trunk/res/res_json.exports.in 383261 
>   /trunk/tests/test_json.c 383261 
> 
> Diff: https://reviewboard.asterisk.org/r/2381/diff
> 
> 
> Testing
> -------
> 
> Ran bouncing testsuite tests 10 times without failure.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130318/258c086d/attachment.htm>