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

[David Lee]

> On March 14, 2013, 11:52 a.m., opticron wrote:
> > /trunk/main/manager_channels.c, line 95
> > <https://reviewboard.asterisk.org/r/2381/diff/3/?file=34483#file34483line95>
> >
> >     This should probably use manager_build_channel_state_string since it's a message being generated from a channel snapshot and it should be processed like the other channel state AMI updates.

Newexten has 'Extension' instead of 'Exten'. Is it really worth making that change?

Newexten also has 'Application' and 'AppData' fields, which aren't currently in the channel_state_string. Should we add it for all events that use that function, or just optionally for Newexten?


On March 14, 2013, 11:52 a.m., David Lee wrote:
> > I'll also make the argument that varset and userevent manager events should use manager_build_channel_state_string and the generic mechanism for generating these events since they pull data from a channel state snapshot.

Varset is only using the uniqueid and channel name. Userevent is only using the uniqueid. Do we want to add the rest of the channel state to those events?


- David


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


On March 14, 2013, 11:07 a.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2381/
> -----------------------------------------------------------
> 
> (Updated March 14, 2013, 11:07 a.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.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/json.c PRE-CREATION 
>   /trunk/main/manager.c 382826 
>   /trunk/main/channel.c 382826 
>   /trunk/configure.ac 382826 
>   /trunk/include/asterisk/autoconfig.h.in 382826 
>   /trunk/include/asterisk/channel.h 382826 
>   /trunk/include/asterisk/manager.h 382826 
>   /trunk/configure 382826 
>   /trunk/apps/app_userevent.c 382826 
>   /trunk/main/manager_channels.c PRE-CREATION 
>   /trunk/main/pbx.c 382826 
>   /trunk/pbx/pbx_realtime.c 382826 
>   /trunk/res/res_json.c 382826 
>   /trunk/res/res_json.exports.in 382826 
>   /trunk/tests/test_json.c 382826 
> 
> 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/20130315/5474beb0/attachment.htm>